On 13 September 2013 19:06, Gary Gregory <[email protected]> wrote:
> Hello David,
>
> Can you test with the 1.4-SNAPSHOT code from trunk? (You'll have to check
> it out and build it).
The method JXPathContextReferenceImpl.getNodePointerFactories() is not
synchronized in trunk either, and the static field is not volatile.
Furthermore, the method returns the array without copying it, so
callers can change entries in it.
Also the field is created with a lock on the class, but is set to null
with a lock on the static feld nodeFactories.
And of course read with no lock at all.
Not good; safe publication requires that reader and writer lock the
same object. And writers must use the same object for mutual
exclusion.
The code is extremely fragile.
> In trunk, NodePointer.java:80) is:
>
> pointer = new NullPointer(name, locale);
>
> which can't throw an NPE.
1.3 has the following code:
NodePointerFactory[] factories =
JXPathContextReferenceImpl.getNodePointerFactories();
for (int i = 0; i < factories.length; i++) { // <= line 80
The for loop is at line 86 in trunk.
> I have not looked at this code in a while, now that multi-core CPUs are
> common place I am not surprised to see issues like this.
>
> I am not even sure why the statics in JXPathContext are lazy loaded (at
> least they are volatile). It would simplify the code to just init the
> statics in-line.
Mutable static fields are generally very bad for thread-safety.
> Thank you,
> Gary
>
>
> On Thu, Sep 12, 2013 at 10:11 PM, David Ferry <[email protected]> wrote:
>
>> Hi
>>
>> We're using Apache JXPath 1.3. We have multi-threaded code running
>> JXPathContext.newContext
>>
>> We're having an occasional NullPointerException coming out of
>> NodePointer.newNodePointer
>>
>> We've had a look at the code for this class, and wonder if
>> getNodePointerFactories is not synchronising correctly.
>>
>> Other operations that read or write the attribute nodeFactoryArray ...
>> synchronize on nodeFactories.
>>
>> But in getNodePointerFactories() ... it just returns the variable without
>> locking.
>> This makes us think that there is no happens-before in the
>> getNodePointerFactories() method.
>>
>> I've put a snippet of the stack trace below.
>>
>> Apologies if this has come up before, I didn't manage to find it after a
>> while searching.
>>
>> Regards,
>> -David
>>
>> java.lang.NullPointerException
>> at
>> org.apache.commons.jxpath.ri.model.NodePointer.newNodePointer(NodePointer.java:80)
>> at
>> org.apache.commons.jxpath.ri.JXPathContextReferenceImpl.<init>(JXPathContextReferenceImpl.java:193)
>> at
>> org.apache.commons.jxpath.ri.JXPathContextReferenceImpl.<init>(JXPathContextReferenceImpl.java:167)
>> at
>> org.apache.commons.jxpath.ri.JXPathContextFactoryReferenceImpl.newContext(JXPathContextFactoryReferenceImpl.java:39)
>> at
>> org.apache.commons.jxpath.JXPathContext.newContext(JXPathContext.java:416)
>>
>
>
>
> --
> E-Mail: [email protected] | [email protected]
> Java Persistence with Hibernate, Second
> Edition<http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
