Bug #2338: saxon:compile-stylesheet() modifies Configuration's ErrorListener - Saxon - Saxonica Developer Community

Actions

Send by e-mail Copy link

Bug #2338

closed

saxon:compile-stylesheet() modifies Configuration's ErrorListener

Added by Anna Benton over 9 years ago. Updated over 8 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Michael Kay

Category:

Saxon extensions

Sprint/Milestone:

Start date:

2015-03-24

Due date:

% Done:

100%

Estimated time:

Legacy ID:

Applies to branch:

9.5, 9.6

Fix Committed on Branch:

9.5, 9.6

Fixed in Maintenance Release:

9.5.1.10, 9.6.0.6

Platforms:

Description

When we get an error in DocumentBuilder.build() it's using the ErrorListener of another thread to report it.

Details:

Document builder part:

We create a SAXSource, passing in an XMLReader on which we have set an ErrorHandler.

There's a problem with the source file (it is empty) and the ErrorHandler is used to output an error. At the same time an error goes out to an ErrorListener for a completely different thread. I poked around the DocumentBuilder source a little and found that DocumentBuilder.build() calls buildDocument from the Configuration, and that has a note that says:

"if any errors occur during document parsing or validation. Detailed errors occurring during schema validation will be written to the ErrorListener associated with the AugmentedSource, if supplied, or with the Configuration otherwise."

The transform part:

While we are failing to build a document with one thread we are successfully transforming some xml in another. When we do this we create a new XsltTransformer off of a cached XSLTExecutable and we attach an ErrorListener that is specific to that transformation to it. Note that we call load() on this single XsltExecutable across many threads at once to get an XsltTransformer for each xml file in a batch, but we do not re-use the XsltTransformers (although to isolate this problem I limited our inputs to a single xml file being transformed and a single xml file going through DocumentBuilder).

When we actually do our transform() the XsltTransformer's ErrorListener becomes the ErrorListener of the Configuration. I've tracked this by outputting the ERROR_LISTENER_CLASS configuration property from the Processor right before and right after the transform() call.

A clearer example:

Thread 1:

Configuration ErrorListener Originally: net.sf.saxon.lib.StandardErrorListener

Source #1 runs a transform() (with one of our ErrorListeners set on the XsltTransformer)

Configuration ErrorListener is now set to one of our ErrorListeners (it is using our LogWriter class)

Thread 2:

Source #2 is an empty file. When we go to build a document off of it, an error is emitted to two places:

The ErrorHandler we set up on the XMLReader which we pass in when we build the SAXSource (this is great, it's what we want).
The Configuration's ErrorListener, which, thanks to thread #1, is now Source #1's LogWriter.

We depend on our ErrorListeners to write out our logs for our transformations, which we then parse and use.

Are we not using ErrorListeners as intended here? Is there a way to keep the ErrorListener we set on the XsltTransformer from being set on the Configuration? That would be our first choice since we're not sure what other circumstances might trigger its use at the Configuration level. I haven't run through the Saxon code itself very far, so I'm not sure of the exact point at which transforming sets the Configuration's ErrorListener. Note that we're using the EnterpriseConfiguration with saxonEE 9.5.1.8.

Actions

Copy link

Updated by Michael Kay over 9 years ago

Yes, this area is pretty messy. Given a free choice, we wouldn't have an ErrorListener at the Processor/Configuration level, but we need it because our Configuration corresponds to JAXP's TransformerFactory, and JAXP puts the ErrorListener on the TransformerFactory.

The intention is that setting an ErrorListener on the Configuration should implicitly set the ErrorListener on queries and transforms run under that Configuration, but not vice versa. You seem to suggest that it's happening the other way, and I don't immediately see how that can happen.

Complicating this further is that if the ErrorListener implements Saxon's StandardErrorListener interface, then we can clone it using its getAnother() method, so that different instances can be used in different threads.

I'd be grateful if you could put together a repro that illustrates the problem more specifically.

Actions

Copy link

Updated by Anna Benton over 9 years ago

Dear Dr. Kay,

I've sent you a link to a zip file containing our test suite (took us a

while to simplify things) via Google Docs.

Please let me know if you have any questions or if it doesn't work for you!

I forgot to mention in the note on the google doc, we've been running it

with java1.6.

Thanks,

Anna Benton

On Tue, Mar 24, 2015 at 11:09 AM, Saxonica Developer Community <

dropbox+saxonica+f38e@plan.io> wrote:

Actions

Copy link

Updated by Anna Benton over 9 years ago

File ConfigurationErrorListenerBug.zip added

We've managed to reduce our test case even further, removing our ExtensionInstruction entirely. I'm attaching a zip of the more reduced test.

To replicate bug:

Unzip ConfigurationErrorListenerBug.zip and drop a saxon9ee.jar into the lib (didn't want to post our saxon jar with our license embedded in it, we're using saxon 9.5).

Run this from the ConfigurationErrorListenerBug directory (we used java1.6):

java -cp .:./lib/* ErrorListenerTest bug.errorlistener.xsl void.xml

Output to command line should be this:

Configuration's ErrorListener is :: net.sf.saxon.lib.StandardErrorListener

Configuration's ErrorListener after setting ErrorListener on transformer :: net.sf.saxon.lib.StandardErrorListener

Configuration's ErrorListener before transform() :: net.sf.saxon.lib.StandardErrorListener

Configuration's ErrorListener after transform() :: ErrorListenerTest$1

(last line shows the change in the Configuration's ErrorListener from StandardErrorListener to our custom error listener in ErrorListenerTest)

Let me know if you run into any problems generating the error.

Actions

Copy link

Updated by Michael Kay over 9 years ago

File deleted (~~ConfigurationErrorListenerBug.zip~~)

Actions

Copy link

Updated by Michael Kay over 9 years ago

Status changed from New to In Progress

The transformation appears to contain a call on the saxon:compile-stylesheet() extension function. This is creating a new JAXP TransformerFactory over the existing Saxon Configuration, and is calling setErrorListener() on that new factory, to be the run-time ErrorListener for the calling transformation. The ErrorListener is held in the underlying Configuration, and because this Configuration is shared with the parent transformation, the main Configuration's ErrorListener is being overwritten.

Fortunately Saxon has extended TransformerFactory.newTemplates() to allow a CompilerInfo to be supplied, and this allows a local ErrorListener and URIResolver to be supplied for a particular compilation. So we can change the code to:

            TransformerFactoryImpl factory = new TransformerFactoryImpl(context.getConfiguration());

            CompilerInfo info = new CompilerInfo();

            info.setErrorListener(context.getController().getErrorListener());

            info.setURIResolver(context.getController().getURIResolver());

            Templates templates = factory.newTemplates(doc, info);

and this appears to fix the problem.

That's for 9.5. The logic of saxon:compile-stylesheet() has changed considerably in 9.6, but looking at the code, I suspect there might be different problems. I'll investigate those now.

Actions

Copy link

Updated by Michael Kay over 9 years ago

Status changed from In Progress to Resolved

For 9.6, saxon:compile-stylesheet() doesn't overwrite the Configuration's ErrorListener and URIResolver, but that's because it makes no attempt to use the run-time ErrorListener and URIResolver for the compilation invoked by compile-stylesheet. I will fix that using similar code to the above.

The 9.7 code is identical to 9.6.

Patch committed for 9.5, 9.6, and 9.7.

Actions

Copy link

Updated by Michael Kay over 9 years ago

Subject changed from Configuration's ErrorListener not thread safe to saxon:compile-stylesheet() modifies Configuration's ErrorListener
Category set to Saxon extensions
Assignee set to Michael Kay

Changed the bug title to reflect the actual cause of the problem (which has nothing to do with thread safety)

Actions

Copy link