9 Replies Latest reply on Aug 29, 2006 9:15 AM by kukeltje

    jBPM 3.1 beta 2 is not thread safe !

    xeha6284

      Hi,

      I have found a problem with createObject method of ObjectFactoryImpl class :

       public Object createObject(ObjectInfo objectInfo) {
       clearRegistry();
       return getObject(objectInfo);
       }
      

      this code is not thread safe

      This can cause errors like

      java.lang.RuntimeException: closed JbpmContext more then once... check your try-finally's around JbpmContexts blocks
      at org.jbpm.JbpmContext.popThisContextFromTheStack(JbpmContext.java:493)
      at org.jbpm.JbpmContext.close(JbpmContext.java:139)

      due to bad context creation when calling JbpmConfiguration.createJbpmContext from a lot of different threads

      It's difficult to produce, so try adding a sleep call after clearRegistry() in createObject method, and you will see the problem ...
       public Object createObject(ObjectInfo objectInfo) {
       clearRegistry();
       try {
       Thread.sleep(2000);
       } catch (InterruptedException e) {
       // TODO Auto-generated catch block
       e.printStackTrace();
       }
       return getObject(objectInfo);
       }
      
      


      I think adding a synchronized will correct the bug.

       public synchronized Object createObject(ObjectInfo objectInfo) {
       clearRegistry();
       return getObject(objectInfo);
       }
      


      Regards,

      Emmanuel

        • 1. Re: jBPM 3.1 beta 2 is not thread safe !
          xeha6284

          I've created an issue on jira (JBPM-544)

          • 2. Re: jBPM 3.1 beta 2 is not thread safe !
            xeha6284

            Hi,

            No answer ?
            Don't you plan to correct this ?

            • 3. Re: jBPM 3.1 beta 2 is not thread safe !
              xeha6284

              It seems to be corrected in release 3.1 : synchronized added to method createObject(String name). So I close this issue

              • 4. Re: jBPM 3.1 beta 2 is not thread safe !

                This is not corrected in the 3.2 CVS?

                jpdl/jar/src/main/java/org/jbpm/configuration/ObjectFactoryImpl.java: public synchronized Object createObject(String name) {
                jpdl/jar/src/main/java/org/jbpm/configuration/ObjectFactoryImpl.java: public Object createObject(int index) {
                jpdl/jar/src/main/java/org/jbpm/configuration/ObjectFactoryImpl.java: public Object createObject(ObjectInfo objectInfo) {

                We have setup an RMI call, and are gettting the closed multiple times error as well.

                James

                • 5. Re: jBPM 3.1 beta 2 is not thread safe !
                  jonabbey

                   

                  "falazar" wrote:
                  This is not corrected in the 3.2 CVS?

                  jpdl/jar/src/main/java/org/jbpm/configuration/ObjectFactoryImpl.java: public synchronized Object createObject(String name) {
                  jpdl/jar/src/main/java/org/jbpm/configuration/ObjectFactoryImpl.java: public Object createObject(int index) {
                  jpdl/jar/src/main/java/org/jbpm/configuration/ObjectFactoryImpl.java: public Object createObject(ObjectInfo objectInfo) {

                  We have setup an RMI call, and are gettting the closed multiple times error as well.

                  James


                  It appears that the problem is that the JbpmConfiguration class uses ThreadLocalStorage for the jbpmContextStacks.. If we attempt to hold a context open and call it from multiple threads, we'll see this error.

                  Since RMI is not designed to map all calls from a client into a single thread, this means we can't reliably call JBPM methods without creating a worker thread on the server and using that for all processing from a single client.

                  I presume something similar is happening on the servlet side, where multiple servlet calls are winding up executing on different threads.

                  I don't understand how this design feature can really work in a J2EE environment. Far better to give each client context its own JbpmContext object, synchronize the methods, and then let anyone call close() on it, without regard to what thread is doing the work.

                  • 6. Re: jBPM 3.1 beta 2 is not thread safe !
                    kukeltje

                     

                    "jonabbey" wrote:

                    It appears that the problem is that the JbpmConfiguration class uses ThreadLocalStorage for the jbpmContextStacks.. If we attempt to hold a context open and call it from multiple threads, we'll see this error.

                    afaik, you are not supposed to keep them open

                    "jonabbey" wrote:

                    Since RMI is not designed to map all calls from a client into a single thread, this means we can't reliably call JBPM methods without creating a worker thread on the server and using that for all processing from a single client.

                    make atomic calls, don't reuse contexts
                    "jonabbey" wrote:

                    I presume something similar is happening on the servlet side, where multiple servlet calls are winding up executing on different threads.

                    Each new request gets a new context

                    "jonabbey" wrote:

                    I don't understand how this design feature can really work in a J2EE environment. Far better to give each client context its own JbpmContext object, synchronize the methods, and then let anyone call close() on it, without regard to what thread is doing the work.

                    Each client context (request) gets its own jbpm context in the webapp. Look at the servletfilters in 3.1 or the jsf jbpmbean in 3.2.

                    If I'm wrong somewhere, let me know. I'm still learning :-)

                    • 7. Re: jBPM 3.1 beta 2 is not thread safe !
                      jonabbey

                       

                      "kukeltje" wrote:
                      "jonabbey" wrote:

                      It appears that the problem is that the JbpmConfiguration class uses ThreadLocalStorage for the jbpmContextStacks.. If we attempt to hold a context open and call it from multiple threads, we'll see this error.

                      afaik, you are not supposed to keep them open


                      I thought jbpmContext was used to mark the transaction boundary? If we can't keep a jbpmContext open long enough to make multiple calls on it, how do we ensure that the operations are done (or not done) in a single transaction?



                      • 8. Re: jBPM 3.1 beta 2 is not thread safe !
                        jonabbey

                         

                        "jonabbey" wrote:
                        "kukeltje" wrote:
                        "jonabbey" wrote:

                        It appears that the problem is that the JbpmConfiguration class uses ThreadLocalStorage for the jbpmContextStacks.. If we attempt to hold a context open and call it from multiple threads, we'll see this error.

                        afaik, you are not supposed to keep them open


                        I thought jbpmContext was used to mark the transaction boundary? If we can't keep a jbpmContext open long enough to make multiple calls on it, how do we ensure that the operations are done (or not done) in a single transaction?



                        Besides, if we want to allow an RMI client to make multiple calls to the server, (even ones that would naturally flow in a single thread sequence in the absence of RMI), RMI *will* spread those calls out on the server over multiple threads. That makes the API rather unusuable, as we'd have to define a separate RMI method for each permuted collection of API calls that we'd want to make between a createContext() call and a close() call.

                        We'd like to be able to have our RMI client make calls to multiple objects on the server in the same sort of natural way that you'd do in a single thread on the server, but we can't guarantee that two successive calls will execute on the same thread.

                        At least, not without doing what we're doing to get around this. We're creating a worker thread and forcing all RMI calls for a session to dispatch through this thread, similar to how the SwingUtils.invokeLater() method works.

                        It would be nice to have a bit more guidance as to how this API is intended to be used, though.

                        • 9. Re: jBPM 3.1 beta 2 is not thread safe !
                          kukeltje

                          Yes, transactionboundary, but within a single thread. There is a discussion about transactions commands etc in the design forum. If you have ideas, tips etc, you are more than welcom to contribute there.