commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [jxpath] race in getNodePointerFactories() ?
Date Sat, 14 Sep 2013 02:04:10 GMT
Weird, now it works.

G


On Fri, Sep 13, 2013 at 5:36 PM, Gary Gregory <garydgregory@gmail.com>wrote:

> Hm... when I build trunk with Maven 3 "mvn clean test" and Java 7 on
> Windows I get:
>
> testLazyProperty(org.apache.commons.jxpath.ri.model.dynabeans.LazyDynaBeanTest)
> Time elapsed: 0.004 sec  <<< ERROR!
> org.apache.commons.jxpath.JXPathNotFoundException: No value for xpath:
> /nosuch
>         at
> org.apache.commons.jxpath.ri.model.NodePointer.verify(NodePointer.java:937)
>         at
> org.apache.commons.jxpath.ri.JXPathContextReferenceImpl.getValue(JXPathContextReferenceImpl.java:374)
>         at
> org.apache.commons.jxpath.ri.JXPathContextReferenceImpl.getValue(JXPathContextReferenceImpl.java:315)
>         at
> org.apache.commons.jxpath.ri.model.dynabeans.LazyDynaBeanTest.testLazyProperty(LazyDynaBeanTest.java:34)
>
> Gary
>
>
> On Fri, Sep 13, 2013 at 5:28 PM, Gary Gregory <garydgregory@gmail.com>wrote:
>
>> On Fri, Sep 13, 2013 at 4:27 PM, sebb <sebbaz@gmail.com> wrote:
>>
>>> On 13 September 2013 19:06, Gary Gregory <garydgregory@gmail.com> 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.
>>>
>>
>> I was not clear: The JXPathContext statics are only writing to by the
>> getters for lazy-init. Why not init them in the decl and make them final?
>>
>> Gary
>>
>>>
>>> > Thank you,
>>> > Gary
>>> >
>>> >
>>> > On Thu, Sep 12, 2013 at 10:11 PM, David Ferry <davidf@opencloud.com>
>>> 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: garydgregory@gmail.com | ggregory@apache.org
>>> > 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: user-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: user-help@commons.apache.org
>>>
>>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> 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
>>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> 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
>



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message