7 Replies Latest reply on Jan 25, 2010 1:21 PM by kukeltje

    jBPM 4.3 - Will cause memory leaks due to unclosed InitialContext()

    jedizippy

      Hi,

       

      Further to my previous post regarding JTA transactions and upon my concern at finding an unclosed InitialContext I did a search through the jBPM source code and to my amazement I found that this is in fact commonplace. And this same issue can be found throughout the a number of classes in the code base.

       

      The following classes have this issue:-

       

      org.jboss.bpm.console.server.util.InvocationProxy

      org.jbpm.jpdl.internal.activity.JavaActivity

      org.jbpm.jpdl.internal.activity.JmsActivity

      org.jbpm.pvm.internal.cfg.ConfigurationImpl

      org.jbpm.pvm.internal.processengine.ProcessEngineImpl

      org.jbpm.pvm.internal.tx.JtaTransaction

      org.jbpm.pvm.internal.wire.descriptor.JbossIdmIdentitySessionFactoryDescriptor

      org.jbpm.pvm.internal.wire.descriptor.JndiDescriptor

       

      This means that the current distribution of 4.3 could simply not be used in a real production scenario due to memory leaks and issues with redployments.


      Regards

      Martin

        • 1. Re: jBPM 4.3 - Will cause memory leaks due to unclosed InitialContext()
          jedizippy

          Created JIRA for this issue JBPM-2763

           

          Regards

          Martin

          • 2. Re: jBPM 4.3 - Will cause memory leaks due to unclosed InitialContext()
            saraswati.santanu

            Martin,

                Did you really faced any OutOfMemoryError because of this?

             

                Typically InitialContext context = new InitialContext() does not take a lot of resources and build a new context altogether. InitialContext just works as a delegate to the actual initial context created using some environment variables/ jndi.properties. When you do a new InitialContext() it asks the InitialContextFactory for the actual InitialContext, and it will be suicidal if the InitialContextFactory creates a new context whenever somebody calls getInitialContext() on it. And I can assure that no proper InitialContextFactory implementation does that. So calling close should not be required at all.

             

                Now what if I call close? Well the code in IntialContext.java is this:

             

                public void close() throws NamingException {
                    myProps = null;
                    if (defaultInitCtx != null) {
                        defaultInitCtx.close();
                        defaultInitCtx = null;
                    }
                    gotDefault = false;
                }

             

                So this closes the defaultInitContext, which is the JNDI context of the server. So if I do so, any further initial context lookup in my application is going to fail !! It will be interesting to see what happens if I call close. It should depend on the implementation and the implementation should understand that the real intention is not to close.

             

            Regards,

            Santanu

            • 3. Re: jBPM 4.3 - Will cause memory leaks due to unclosed InitialContext()
              kukeltje
              What happens on closing is dependent on the implementation I read somewhere... So it indeed might not be a problem at all, but I soal read somewhere that on Weblogic you should close it, but that might also be jusr for remote contexts and not the 'local/server' one. So I agree to want to see proof it is a real issue.
              • 4. Re: jBPM 4.3 - Will cause memory leaks due to unclosed InitialContext()
                jedizippy

                Hi,

                 

                I will look into this and see how it affects our deployment/redeployment. However its normal coding practice to always any close resources after use. For sure Weblogic will not close it for you. Maybe JBoss does (which is actually quite worrying) but you cannot rely on the app server for that and really why would you want to when you can simply add a finally block with a try/catch/close in it. Its basic coding good practice.

                 

                In our case hot deployment and high availability (99.9%) are key issues and as such our architect for example would not allow us to use the Spring Framework for example due to issues with threading/jms tainting the app server (his issues not mine btw). So we will have to patch this code anyway to to make it work for us. In this case I might as well contribute it back to the source branch (as I said I cant see why you would not want do do it) !.

                 

                Having said that we have bigger issues to look at today with the contraint violations as if we can solve those then this is irrelavent anyway !.

                 

                Regards

                Martin

                • 5. Re: jBPM 4.3 - Will cause memory leaks due to unclosed InitialContext()
                  tom.baeyens
                  • 6. Re: jBPM 4.3 - Will cause memory leaks due to unclosed InitialContext()
                    kukeltje
                    I closed it, since it is a duplicate: http://community.jboss.org/message/521698#521698
                    • 7. Re: jBPM 4.3 - Will cause memory leaks due to unclosed InitialContext()
                      kukeltje

                      I still would like to see if it is realy a problem. I agree that you should close a context if you use a remote one, or a specially created one with properties, but in case you use the default platform one it might cause problems.

                       

                      And the one mentioned in http://community.jboss.org/thread/147161?tstart=0 is used in a static method. What is the difference using

                       

                       

                      InitialContext initialContext = new InitialContext()
                      initialContext.lookup(....)
                      
                      

                       

                      In a static method and using

                       

                      InitialContext.doLookup(...)
                      
                      

                       

                      Which internally does (not surprisingly)

                       

                      return new InitialContext().lookup(....)
                      

                       

                       

                      You cannot even close it then... So...

                       

                      Yet, I'm not an expert in this area....