Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

      Description

      I was unable to determine if JXPath was intended to be used concurrently or not,
      but after using it in a server application that runs 100+ threads concurrently,
      I started getting errors (they were rare) that showed the following trace:

      java.lang.NullPointerException
      at
      org.apache.commons.jxpath.ri.model.beans.BeanPropertyPointer.getPropertyNames(BeanPropertyPointer.java:72)
      at
      org.apache.commons.jxpath.ri.model.beans.PropertyIterator.prepareForIndividualProperty(PropertyIterator.java:248)
      at
      org.apache.commons.jxpath.ri.model.beans.PropertyIterator.setPositionIndividualProperty(PropertyIterator.java:141)
      at
      org.apache.commons.jxpath.ri.model.beans.PropertyIterator.setPosition(PropertyIterator.java:127)
      at
      org.apache.commons.jxpath.ri.axes.ChildContext.setPosition(ChildContext.java:106)
      at org.apache.commons.jxpath.ri.axes.ChildContext.nextNode(ChildContext.java:89)
      at org.apache.commons.jxpath.ri.EvalContext.nextSet(EvalContext.java:322)
      at org.apache.commons.jxpath.ri.axes.UnionContext.setPosition(UnionContext.java:55)
      at
      org.apache.commons.jxpath.ri.axes.NodeSetContext.nextNode(NodeSetContext.java:64)
      at org.apache.commons.jxpath.ri.EvalContext.constructIterator(EvalContext.java:181)
      at org.apache.commons.jxpath.ri.EvalContext.hasNext(EvalContext.java:114)
      at
      com.paychex.hrs.ei.conversion.jaxb.InboundProcessor.process(InboundProcessor.java:136)

      The code calling has next looks like:
      Iterator oIter = m_oIdentifier.iteratePointers(oXCtx);
      >>> while (oIter.hasNext())
      {

      The application has 100 worker threads, each processing the same
      message(converted to beans by JAXB), but different instances of it. This error
      occurs infrequently - happening only about once every 3 runs (each run processes
      1000 messages). I performed the test on a P4/Windows box, but I believe that
      this is independent of os/system.

      I believe I have found some of the source code that may be causing this. It
      appears that bean info is stored in an internal cache (JXPathIntrospector) that
      all threads would end up using. For this to be thread safe, JXPathIntrospector
      needs to be thread safe (which it is even though synchronization is not used),
      and JXPathBasicBeanInfo needs to be thread safe (it is not). Basically what is
      happening is that inside of JXPathBasicBeanInfo, a couple of procedures build
      member variables on demand. However this demand based building is not thread
      safe and so a race condition exists between multiple threads that are both
      performing the on demand building. Specifically getPropertyDescriptors and
      getPropertyDescriptor modify propertyDescriptors and propertyNames in an unsafe
      manner. Either synchronization should be used or the assignment to the member
      variable should be performed last (resulting in duplicate effort but thread
      safety). Here are the pieces of code I am referring to:

      public PropertyDescriptor[] getPropertyDescriptors() {
      if (propertyDescriptors == null) {
      try {
      BeanInfo bi = null;
      if (clazz.isInterface())

      { bi = Introspector.getBeanInfo(clazz); }

      else

      { bi = Introspector.getBeanInfo(clazz, Object.class); }

      PropertyDescriptor[] pds = bi.getPropertyDescriptors();
      propertyDescriptors = new PropertyDescriptor[pds.length];
      // At this point, the cache of property descriptors has been cleared possibly
      // conflicting with other threads
      System.arraycopy(pds, 0, propertyDescriptors, 0, pds.length);
      Arrays.sort(propertyDescriptors, new Comparator() {
      public int compare(Object left, Object right)

      { return ((PropertyDescriptor) left).getName().compareTo( ((PropertyDescriptor) right).getName()); }

      });
      }
      ...

      public PropertyDescriptor getPropertyDescriptor(String propertyName) {
      if (propertyNames == null) {
      PropertyDescriptor[] pds = getPropertyDescriptors();
      propertyNames = new String[pds.length];
      // At this point the property names has been cleared possibly conflicting with
      // other threads
      for (int i = 0; i < pds.length; i++)

      { propertyNames[i] = pds[i].getName(); }

      }

      Thanks,
      Rob Sax

        Issue Links

          Activity

          Hide
          Dmitri Plotnikov added a comment -

          Per Rob's suggestions changed the code to make the field assignment last

          Show
          Dmitri Plotnikov added a comment - Per Rob's suggestions changed the code to make the field assignment last

            People

            • Assignee:
              Unassigned
              Reporter:
              Robert Sax
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development