'Re: [jxpath] race in getNodePointerFactories() ?'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       jakarta-commons-user
Subject:    Re: [jxpath] race in getNodePointerFactories() ?
From:       Gary Gregory <garydgregory () gmail ! com>
Date:       2013-09-14 15:16:49
Message-ID: CACZkXPyAK8bziz_0EbmFgV=052rLLGsJq07seL6LChG9WZ043A () mail ! gmail ! com
[Download RAW message or body]


Check. Let's do 1.5 in trunk at least then.

Gary


On Sat, Sep 14, 2013 at 8:22 AM, sebb <sebbaz@gmail.com> wrote:

> On 14 September 2013 12:54, Gary Gregory <garydgregory@gmail.com> wrote:
> > Maybe we should set a Java requirement for Jxpath 1.4? I would just
> > use Java 6 as a min for now. Or why not 7?
> 
> I think 1.5 would be a more sensible base.
> 
> That gives big advantages over 1.4 and 1.3 without potentially
> excluding users stuck on 1.5.
> 
> There is no point requiring our users to use 1.7 (or 1.6) unless the
> code *requires* it.
> 
> 1.6 has some additional useful features (mainly Charset), but in
> comparison with 1.4 => 1.5 is not a huge advance.
> 
> > Gary
> > 
> > On Sep 14, 2013, at 5:17, sebb <sebbaz@gmail.com> wrote:
> > 
> > > Tests OK for me as well using Java 1.5.
> > > [I no longer have 1.3 or 1.4 installed]
> > > 
> > > BTW, the pom does not have any compiler source or target settings so
> > > defaults to 1.3 (which is why it uses JUnit 3.8.1).
> > > 
> > > On 14 September 2013 03:04, Gary Gregory <garydgregory@gmail.com>
> wrote:
> > > > 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
> > > 
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: user-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: user-help@commons.apache.org
> > > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: user-unsubscribe@commons.apache.org
> > For additional commands, e-mail: user-help@commons.apache.org
> > 
> 
> ---------------------------------------------------------------------
> 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



[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic