Bug #3979: Synchronization of xsl:message - Saxon - Saxonica Developer Community

Actions

Send by e-mail Copy link

Bug #3979

closed

Synchronization of xsl:message

Added by Michael Kay about 6 years ago. Updated about 6 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Michael Kay

Category:

Multithreading

Sprint/Milestone:

Start date:

2018-10-23

Due date:

% Done:

100%

Estimated time:

Legacy ID:

Applies to branch:

9.8, 9.9

Fix Committed on Branch:

9.8, 9.9

Fixed in Maintenance Release:

9.8.0.15, 9.9.0.2

Platforms:

Description

Because all xsl:message output within a transformation is sent to the same MessageEmitter, and because the MessageEmitter will generally not be thread-safe, xsl:message synchronizes itself on the message emitter so that a transformation can only be executing one xsl:message instruction at any one time. This is only a problem, of course, if xsl:message is used within multi-threading constructs such as xsl;result-document or a multi-threading xsl:for-each instruction.

The synchronization causes a number of problems identified in thread https://saxonica.plan.io/boards/3/topics/7336

It can cause contention and hence negate any benefits of multi-threading
Holding a synchronization lock while executing potentially long-running user-written code is intrinsically bad for performance and creates a risk of deadlock.
One can probably create code that will inevitably deadlock, for example by using a multi-threading construct within the body of the xsl:message instruction.

Possible solutions include (a) buffering the content of the xsl:message output in memory, and holding the synchronization lock only while writing the output to the MessageEmitter, and (b) disabling multi-threading constructs such as xsl:for-each with the saxon:threads attribute while evaluating xsl:message.

Note: perhaps surprisingly, xsl:message does not set temporary output state, so it is possible to execute xsl:result-document while executing xsl:message.

Actions

Copy link

Updated by Michael Kay about 6 years ago

It's quite tricky to solve this without API change, because the current XsltController.setMessageEmitter() interface assumes that the entire transformation uses a single message emitter, and the message emitter (being a Receiver) will not be thread-safe.

On the other hand, there's a bit of a history of problems with the message emitter, some of them unresolved (see for example #3978, #3620, #2443, #2243) and some redesign might be overdue. API changes at this level on the 9.9 branch are probably still acceptable.

I would suggest an API where the application supplies a MessageReceiverFactory - a class that we call once for every xsl:message invocation, which returns a Receiver, to which we write events representing the content of that message. The close() method of the Receiver takes responsibility for sending the message content to some appropriate location. This approach means that evaluating xsl:message no longer needs to be synchronized. The standard message receiver will serialize the message, and its close() method will append the serialized message content to the message destination (supplied by another method) - this write() action will be synchronized.

Actions

Copy link

Updated by Michael Kay about 6 years ago

Status changed from New to In Progress

I have (experimentally) made the following changes on the 9.9 branch:

XsltController.setMessageEmitter() and getMessageEmitter() are deprecated and have no useful effect.
XsltController acquires new methods setMessageFactory() and getMessageFactory(). The message factory is a Supplier<Receiver> that is called to get a new message receiver every time xsl:message is evaluated.
XsltController.setMessageReceiverClassName() is retained, and calls setMessageFactory() with a factory class that instantiates the supplied class.
XsltController.new() sets a default message factory of MessageListener::new. The MessageListener class is unchanged.
Message (the class that implements the xsl:message instruction) now gets a new message receiver for every message, and no longer synchronizes.
At the s9api level, AbstractXsltTransformer retains the methods setMessageListener() and getMessageListener() (both variants, accepting a MessageListener or MessageListener2); the implementation of these changes so that they set a message factory on the underlying XsltController.

The XSLT3 xsl:message tests are working with these changes (and with no change to the test driver); I now need to look at xsl:message unit tests.

Actions

Copy link

Updated by Michael Kay about 6 years ago

To reduce the compatibility impact, I've reinstated XsltController.setMessageEmitter() provided that multithreading is disabled (which will always be the case in Saxon-HE). I think this provides an adequate solution for 9.9.

Now need to think about a fix for the 9.8 branch, with even less backwards compatibility impact.

Actions

Copy link

Updated by Michael Kay about 6 years ago

Status changed from In Progress to Resolved
Applies to branch 9.8, 9.9 added
Fix Committed on Branch 9.8, 9.9 added

The changes previously made for the 9.9 branch are confirmed.

For 9.8 I have changed xsl:message so that, if multithreading is permitted by the configuration, the output of a single call on xsl:message is buffered in a local SequenceOutputter, and copied to the MessageEmitter (under synchronization) on completion. This avoids holding a synchronisation lock while executing user-written code.

Actions

Copy link