JBAS-3428 - Duplication on Proxies

[clebert.suconic]

MBeanProxyExt.create is creating a new ClassLoader and forcing a loadClass to return an instance of a proxy using that classLoader.

I guess this is done to make sure a redeployment leakage is not going to happen.

IMO it would be better just use the original ClassLoader, and make sure applications are releasing instances to the proxy. Creating a new ClassLoader here would just mascarade a real problem, and besides create a worse leakage.

On the case of JBAS-3428 I guess we should just revert to MBeanProxy.get. (I would like to hear from others), and if we see a redeployment leakage we should clear references to its proxies.

I also thinkg MBeanProxyExt.create should behave the same way as MBeanProxy.get. Just IMHO.

Clebert Suconic

[starksm64]

Explain how this is causing memory leaks.

[clebert.suconic]

On a EntityBean.remove call, EntityContainer.remove() is calling Container.removeTimerService()

Container.removeTimerService() is calling SecurityActions.getMBeanProxy...

At that point getMBeanProxy is calling MBeanProxyExt.create() that is creating a classLoader and a proxy for every call. What means every EntityBean.remove() is creating a new Proxy.

Fixing SecurityActions.getMBeanProxy to use MBeanProxy.get() instead fixed the problem.

Here is the stack trace from the SpecjTest we were executing:

Pay attention to the name of the proxy ($Proxy18528). I have a GC LOG at the end of this test unloading 40K Proxy classes.

"http-192.168.104.7-8080-503" daemon prio=10 tid=0x02273630 nid=0x36c runnable [0xdcd7c000..0xdcd7f8f0]
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:164)
at $Proxy18528.<clinit>(Unknown Source)
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
at java.lang.reflect.Constructor.newInstance(Constructor.java:494)
at java.lang.reflect.Proxy.newProxyInstance(Proxy.java:588)
at org.jboss.mx.util.MBeanProxyExt.create(MBeanProxyExt.java:395)
at org.jboss.mx.util.MBeanProxyExt.create(MBeanProxyExt.java:349)
at org.jboss.ejb.SecurityActions.getMBeanProxy(SecurityActions.java:148)
at org.jboss.ejb.Container.removeTimerService(Container.java:739)
at org.jboss.ejb.EntityContainer.remove(EntityContainer.java:539)
at sun.reflect.GeneratedMethodAccessor282.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:585)
at org.jboss.invocation.Invocation.performCall(Invocation.java:359)
at org.jboss.ejb.EntityContainer$ContainerInterceptor.invoke(EntityContainer.java:1176)
at org.jboss.ejb.plugins.cmp.jdbc2.RelationInterceptor.invoke(RelationInterceptor.java:67)
at org.jboss.ejb.plugins.EntitySynchronizationInterceptor.invoke(EntitySynchronizationInterceptor.java:284)
at org.jboss.resource.connectionmanager.CachedConnectionInterceptor.invoke(CachedConnectionInterceptor.java:158)
at org.jboss.ejb.plugins.EntityReentranceInterceptor.invoke(EntityReentranceInterceptor.java:126)
at org.jboss.ejb.plugins.EntityInstanceInterceptor.invoke(EntityInstanceInterceptor.java:276)
at org.jboss.ejb.plugins.EntityCreationInterceptor.invoke(EntityCreationInterceptor.java:68)
at org.jboss.ejb.plugins.AbstractTxInterceptor.invokeNext(AbstractTxInterceptor.java:121)
at org.jboss.ejb.plugins.TxInterceptorCMT.runWithTransactions(TxInterceptorCMT.java:350)
at org.jboss.ejb.plugins.TxInterceptorCMT.invoke(TxInterceptorCMT.java:181)
at org.jboss.ejb.plugins.SecurityInterceptor.invoke(SecurityInterceptor.java:168)
at org.jboss.ejb.plugins.LogInterceptor.invoke(LogInterceptor.java:205)
at org.jboss.ejb.plugins.ProxyFactoryFinderInterceptor.invoke(ProxyFactoryFinderInterceptor.java:136)
at org.jboss.ejb.EntityContainer.internalInvoke(EntityContainer.java:520)
at org.jboss.ejb.Container.invoke(Container.java:954)
at org.jboss.ejb.plugins.local.BaseLocalProxyFactory.invoke(BaseLocalProxyFactory.java:430)
at org.jboss.ejb.plugins.local.EntityProxy.invoke(EntityProxy.java:65)
at $Proxy122.remove(Unknown Source)
at org.jboss.ejb.plugins.cmp.jdbc2.bridge.JDBCCMRFieldBridge2$CollectionValuedFieldState.cascadeDelete(JDBCCMRFieldBridge2.java:908)
at org.jboss.ejb.plugins.cmp.jdbc2.bridge.JDBCCMRFieldBridge2.remove(JDBCCMRFieldBridge2.java:281)
at org.jboss.ejb.plugins.cmp.jdbc2.bridge.JDBCEntityBridge2.remove(JDBCEntityBridge2.java:341)
at org.jboss.ejb.plugins.cmp.jdbc2.JDBCStoreManager2.removeEntity(JDBCStoreManager2.java:415)
at org.jboss.ejb.plugins.CMPPersistenceManager.removeEntity(CMPPersistenceManager.java:521)
at org.jboss.resource.connectionmanager.CachedConnectionInterceptor.removeEntity(CachedConnectionInterceptor.java:314)
at org.jboss.ejb.EntityContainer.remove(EntityContainer.java:536)
at sun.reflect.GeneratedMethodAccessor282.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:585)
at org.jboss.invocation.Invocation.performCall(Invocation.java:359)
at org.jboss.ejb.EntityContainer$ContainerInterceptor.invoke(EntityContainer.java:1176)
at org.jboss.ejb.plugins.cmp.jdbc2.RelationInterceptor.invoke(RelationInterceptor.java:67)
at org.jboss.ejb.plugins.EntitySynchronizationInterceptor.invoke(EntitySynchronizationInterceptor.java:284)
at org.jboss.resource.connectionmanager.CachedConnectionInterceptor.invoke(CachedConnectionInterceptor.java:158)
at org.jboss.ejb.plugins.EntityReentranceInterceptor.invoke(EntityReentranceInterceptor.java:126)
at org.jboss.ejb.plugins.EntityInstanceInterceptor.invoke(EntityInstanceInterceptor.java:276)
at org.jboss.ejb.plugins.EntityCreationInterceptor.invoke(EntityCreationInterceptor.java:68)
at org.jboss.ejb.plugins.AbstractTxInterceptor.invokeNext(AbstractTxInterceptor.java:121)
at org.jboss.ejb.plugins.TxInterceptorCMT.runWithTransactions(TxInterceptorCMT.java:350)
at org.jboss.ejb.plugins.TxInterceptorCMT.invoke(TxInterceptorCMT.java:181)
at org.jboss.ejb.plugins.SecurityInterceptor.invoke(SecurityInterceptor.java:168)
at org.jboss.ejb.plugins.LogInterceptor.invoke(LogInterceptor.java:205)
at org.jboss.ejb.plugins.ProxyFactoryFinderInterceptor.invoke(ProxyFactoryFinderInterceptor.java:136)
at org.jboss.ejb.EntityContainer.internalInvoke(EntityContainer.java:520)
at org.jboss.ejb.Container.invoke(Container.java:954)
at org.jboss.ejb.plugins.local.BaseLocalProxyFactory.invoke(BaseLocalProxyFactory.java:430)
at org.jboss.ejb.plugins.local.LocalHomeProxy.invoke(LocalHomeProxy.java:121)
at $Proxy81.remove(Unknown Source)

[starksm64]

There is no reason to be creating a timer service proxy on every usage as is currently being done. The current situation seems to be excessive object creation, not a leak which is why the change was made in the first place.

[dimitris]

From the top of my head, I'm sure we have plenty of places in the code we create a proxy for a single call, and throw it away. It's a bad practise.

[clebert.suconic]

OK, I knew technically is not a leakage by definition, but in practice, in our tests, everything is pegged because of this.

These proxies are alive during the whole cycle of the test, and released at the end. We have seen OME sometimes also.

Also... Are you sure MBeanProxyExt would avoid a redeployment leakage?

The Class created will have a reference back to the fake classLoader, what will have a reference to the parent classLoader, what would hang the classLoader anyways.

[clebert.suconic]

From the top of my head, I'm sure we have plenty of places in the code we create a proxy for a single call, and throw it away. It's a bad practise.

We will have to clean these references during undeployment, or to use SoftReferences testing for Null Object.

I think the JVM doesn't make any internal references to the created proxy, what means the Proxy will released if a GC happens.

On JBoss Serialization, I created a package called PersistentReference that encapsulates such thing. It's really simple.

If we want to cache proxies without having to worry on clearing references later maybe we should implement a similar idea.

[starksm64]

Also... Are you sure MBeanProxyExt would avoid a redeployment leakage?

The Class created will have a reference back to the fake classLoader, what will have a reference to the parent classLoader, what would hang the classLoader anyways.

The reference to the parent should not hold anything as the proxy is created by the child class loader. Unless there is an outside reference to the proxy, the proxy and the class loader created by MBeanProxyExt would be available for gc.

[clebert.suconic]

The reference to the parent should not hold anything as the proxy is created by the child class loader. Unless there is an outside reference to the proxy, the proxy and the class loader created by MBeanProxyExt would be available for gc.

If there is a reference to the created proxy, the classLoader is referenced anyway.
The child classLoader (proxyInstance.getClass().getClassLoader()) will have a reference to the classLoader passed to the constructor. (parent classLoader).

So, you still need to release the reference to the created proxy if you want to not hold redeployments. And if you still need to clean the reference why not just use MBeanProxy.get()?

[starksm64]

You always have to clear the proxy. Drill down into the point of the change described in JBAS-3013 with Adrian. The fix in this issue is to not be creating the timer service proxy on every usage.

[clebert.suconic]

Yes... you're right... Adrian changed lots of places caching the proxy.
org.jboss.ejb.Container would have to be rationalized also.

Also, I still don't like the ClassLoader being created inside MBeanExt.create(), but that's just my personal preference ;-) . (this is just IMO).