[ 
https://issues.apache.org/jira/browse/XERCESJ-1377?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12717227#action_12717227
 ] 

Peter Geraghty commented on XERCESJ-1377:
-----------------------------------------

In terms of verifying the fix, I've looked at the new revision and it looks OK, 
but unfortunately I don't have a way of verifying the solution in practice, for 
the following reasons:

So far we have not actually managed to reproduce this problem in our own office 
on our development machines, which are not the same as the customer's set up.

In the deployment at our customer site, Xerces 2.9.0 is being used, which we 
incorporate as the pre-built impl.jar pulled down via maven dependencies.  
Since the system is in production I am not going to recommend a switch to a 
locally patched and built implementation based on the trunk.  We have patched 
our own calling code to add extra synchronization around our calls to 
"matches", which we will remove when this fix is released and we move our 
dependencies onwards.

If we do recreate the problem on our machines I will try out the fix here.

Thanks for your help on this issue.



> Thread safety problem in RegularExpression
> ------------------------------------------
>
>                 Key: XERCESJ-1377
>                 URL: https://issues.apache.org/jira/browse/XERCESJ-1377
>             Project: Xerces2-J
>          Issue Type: Bug
>          Components: XML Schema 1.0 Datatypes
>    Affects Versions: 2.9.0, 2.9.1
>         Environment: Problem encountered onmulti-processor Linux box, unknown 
> hardware, JVM was Sun JVM:
> java version "1.5.0_10"
> Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0_10-b03)
> Java HotSpot(TM) 64-Bit Server VM (build 1.5.0_10-b03, mixed mode)
>            Reporter: Peter Geraghty
>            Assignee: Michael Glavassevich
>
> String which should have matched pattern was incorrectly reported as not 
> matching.  On re-submission the same string was correctly reported as 
> matching.  This has now happened twice in the first few daysof use of a 
> system processing around 500K messages per day, i.e., problem is highly 
> intermittent.
> The symptoms appear to indicate a thread safety problem and although I had 
> understood RegularExpression to be thread-safe, looking at the code it does 
> seem to be wrong.
> The application in question is using 2.9.0 but looking at 
> RegularExpression.java in 2.9.1 the algorithm appears the same and is 
> described below.
> The thread safety algorithm depends on a separate Context object being 
> allocated on the stack if the Context object  referenced by 
> RegularExpression's instance variable is "inuse".  For example, line 1420.
>         synchronized (this.context) {
>             con = this.context.inuse ? new Context() : this.context;
>             con.reset(target, start, end, this.numberOfClosures);
>         }
> The "inuse" boolean is set to true by the reset method inside the 
> synchronized code section above, however it is set to false in a 
> non-synchronized section, prior to each return point, e.g., at line 1449.
>                 con.inuse = false;
>                 return true;
> The "inuse" boolean is not declared as volatile, and so I believe the absence 
> of synchronization is wrong and makes this class NOT thread safe.
> E.g., it is vulnerable to what the JLS second edition called a "Prescient 
> Store" optimisation taking place, which could explain the behaviour I am 
> seeing - the inuse of this.context being set to false earlier than would be 
> expected could lead to concurrent use of a Context object which is not thread 
> safe.  Since all return points from methods like "matches" do set "inuse" to 
> false, a "prescient store" optimisation to set it to false before actually 
> performing the match is quite plausible.
> Although the term "prescient store" is not used in the JLS third edition I 
> believe the semantics described there for non-volatile field access in 
> non-synchronized code regions still allow this possibility of an optimisation 
> re-ordering the clearing of the inuse flag so that it happens BEFORE the 
> actual use of the Context object.
> In terms of a solution, one possibility is to declare "inuse" as volatile, 
> another is to use a synchronized "setInUse" method on the Context.
> A third possibility would be to dispense with the approach of re-using 
> Context objects via an instance variable reference, and always allocate a 
> Context on the stack.  I also note that if this was done, and if the 
> "prepare" method was not invoked lazily on the first match but was invoked up 
> front as part of setting the pattern, there would be no need for any kind of 
> synchronization within the "matches" methods.  This could give the optimum 
> for heavy concurrent use of a common pattern in highly-multithreaded 
> environment, but of course has trade-offs in other regards.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to