3 Replies Latest reply on Sep 18, 2008 1:40 PM by chiba

    Making delegators thread-safe

    cpopetz

      Hi,

      I'm experiencing an exception under heavy load using hibernate, which I believe is a javassist bug.

      The code that ProxyFactory.makeForwarder generates to invoke the handler is:

       * if (methods[index * 2] == null) {
       * methods[index * 2]
       * = RuntimeSupport.findMethod(this, <overridden name>, <desc>);
       * methods[index * 2 + 1]
       * = RuntimeSupport.findMethod(this, <delegator name>, <desc>);
       * or = null // the original method is abstract.
       * }
       * return ($r)handler.invoke(this, methods[index * 2],
       * methods[index * 2 + 1], $args);
      
      


      This is problematic, because two threads could the same proxy in close enough succession that handler.invoke() could be called with a null third argument:

      Thread a: methods[index *2] = ...;
      Thread b: if (methods[index * 2] == null) //will evaluate to false
      Thread b: loads a null methods[index *2 + 1], invokes handler
      Thread a: methods[index *2 + 1] = ...;

      and we get a NPE along the lines of:

      java.lang.NullPointerException
       at javassist.util.proxy.RuntimeSupport$DefaultMethodHandler.invoke(RuntimeSupport.java:37)
       at edu.academyart.model.Post_$$_javassist_2.getTextTitles(Post_$$_javassist_2.java)
      



      I think this can be fixed by simply reversing the order that the methods are stored in the array, which ensures both exist if the if() block is skipped, as in the below patch.

      $ cvs diff -c2p src/main/javassist/util/proxy/ProxyFactory.java
      Index: src/main/javassist/util/proxy/ProxyFactory.java
      ===================================================================
      RCS file: /cvsroot/jboss/javassist/src/main/javassist/util/proxy/ProxyFactory.java,v
      retrieving revision 1.26
      diff -c -2 -p -r1.26 ProxyFactory.java
      *** src/main/javassist/util/proxy/ProxyFactory.java 2 Oct 2007 03:49:11 -0000 1.26
      --- src/main/javassist/util/proxy/ProxyFactory.java 27 Dec 2007 02:47:28 -0000
      *************** public class ProxyFactory {
      *** 846,851 ****
       code.addIndex(0);
      
      - callFindMethod(code, "findSuperMethod", arrayVar, origIndex, meth.getName(), desc);
       callFindMethod(code, "findMethod", arrayVar, delIndex, delegatorName, desc);
      
       int pc2 = code.currentPc();
      --- 846,851 ----
       code.addIndex(0);
      
       callFindMethod(code, "findMethod", arrayVar, delIndex, delegatorName, desc);
      + callFindMethod(code, "findSuperMethod", arrayVar, origIndex, meth.getName(), desc);
      
       int pc2 = code.currentPc();
      
      


      Please let me know if this seems reasonable, and I'll open a JIRA issue with a patch.