Weird, now it works. G
On Fri, Sep 13, 2013 at 5:36 PM, Gary Gregory <[email protected]>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 <[email protected]>wrote: > >> On Fri, Sep 13, 2013 at 4:27 PM, sebb <[email protected]> wrote: >> >>> 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. >>> >> >> 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 <[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] >>> >>> >> >> >> -- >> 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 >> > > > > -- > 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 > -- 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
