I don't think we should call remove() because we need to reset it back to the previous caller in the component stack. For example, Component A --> Component B --> Component C, A set msg1, B sets msg2, C sets msg3, then C resets to msg2 and B resets to msg1 and A should set it to null.

The memory leak is probably from the first caller as it creates a default value.

Thanks,
Raymond
--------------------------------------------------
From: "Simon Laws" <[email protected]>
Sent: Thursday, July 23, 2009 1:25 PM
To: <[email protected]>
Subject: Re: [1.x] memory leak in binding.jsonrpc - some help required

Thanks for the summary Raymond.


Are you saying the thread local variable is not released? I searched the
usages of the ThreadMessageContext  in our code, all 3 cases are like:

Yes. I don't see a ThreadLocal.remove() in the codebase. I'm guessing
but thread pooling the the container could result in the MessageImpl
instance sittings on the thread with a reference, via its class,  to
the WebAppClassloader after the app has been stoppped. Hence the
classloader will not be GCd and we blow perm gen.


Message msgContext = ThreadMessageContext.getMessageContext(); // Get
existing value [1]
...
ThreadMessageContext.setMessageContext(msg); // Set to new value [2]
try {
...
} finally {
ThreadMessageContext.setMessageContext(msgContext); // Reset to old value
[3]
}

But [1] has some issues because it creates default value (I don't know why
it is not null).
     protected synchronized Message initialValue() {
          Message msg =  new MessageImpl();
          msg.setFrom(new EndpointReferenceImpl("/"));
          return msg;
      }

Is the dummy message leaking?

Not sure. I think it's just whatever we put in thread local.

I'll trying adding a "remove" to the ThreadMessageContext interface
and calling that when the webapp goes down.


BTW, we are also missing a check in
org.apache.tuscany.sca.core.context.impl.CallableReferenceImpl.resolve().

else if (compositeActivator == null) { // Only get the composite activator when it is not set, the current code always looks for the thread local which
is not performed
          this.compositeActivator =
CompositeContext.getCurrentCompositeActivator();
          if (this.compositeActivator != null) {
              this.proxyFactory =
this.compositeActivator.getCompositeContext().getProxyFactory();
          }
      }

We should fix that too.

Reply via email to