MyFaces Core
  1. MyFaces Core
  2. MYFACES-3510

Application components classloader memory leak

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.6
    • Fix Version/s: 2.0.13, 2.1.7
    • Component/s: General
    • Labels:
      None
    • Environment:
      WIndows Glassfish Embedded

      Description

      We've seen an application classloader memory leak due to the new class javax.faces.component._PropertyDescriptorHolder class introduced in version 2.1.6

      This class holds a reference to a Method of a component loaded by the application classloader. The memory leak shows up when the _PropertyDescriptorHolder is stored in the _ComponentAttributesMap class that is loaded by the system class loader.

      You should use a WeakReference instead of storing the direct reference to the Method

      1. MYFACES-3510-2.patch
        4 kB
        Leonardo Uribe
      2. MYFACES-3510-1.patch
        2 kB
        Leonardo Uribe
      3. ASF.LICENSE.NOT.GRANTED--screenshot-1.jpg
        289 kB
        Ruben Martin Pozo

        Issue Links

          Activity

          Hide
          Leonardo Uribe added a comment -

          No, it is not a bug. There is no other way to detect tomcat different to check if such class exists on the classpath. Instead, what do you need is implement a LifecycleProvider2 for glassfish or a custom LifecycleProviderFactory, to skip TomcatAnnotationLifecycleProvider.isAvailable. Unfortunately, since glassfish is CDDL/GPL licensed, we can't include that integration code in MyFaces. See:

          https://cwiki.apache.org/confluence/display/MYFACES/Integration+with+application+and+web+servers

          for more details. It could be good to share your findings later in a blog, other users have reported problems setting up myfaces with glassfish, and maybe that could help

          Show
          Leonardo Uribe added a comment - No, it is not a bug. There is no other way to detect tomcat different to check if such class exists on the classpath. Instead, what do you need is implement a LifecycleProvider2 for glassfish or a custom LifecycleProviderFactory, to skip TomcatAnnotationLifecycleProvider.isAvailable. Unfortunately, since glassfish is CDDL/GPL licensed, we can't include that integration code in MyFaces. See: https://cwiki.apache.org/confluence/display/MYFACES/Integration+with+application+and+web+servers for more details. It could be good to share your findings later in a blog, other users have reported problems setting up myfaces with glassfish, and maybe that could help
          Hide
          Ruben Martin Pozo added a comment -

          I've run the same test with patch2 and the problem keeps happening. The server freezes for 40 seconds in the method TomcatAnnotationLifecycleProvider.isAvailable.

          I run a little test. I changed that method to always return false since my application is running on a glassfish not a tomcat. And magically everything worked!!!. The classloaders are garbage collected and the server doesn't freeze.

          Here is what I think is happening. It might sound science fiction but here it goes:

          I'm running with the flag -XX:+UseConcMarkSweepGC so I'm running a concurrent garbage collector. When there isn't any more room in the permGen for classes, a permGen gc cycle is run. While running this cycle the method TomcatAnnotationLifecycleProvider.isAvailable is trying to know if tomcat is available. To do so, it tries to load a tomcat class. But I don't have this class in my classpath since I'm not running on a tomcat. The JVM looks for the class in the entire classpath but it doesn't find it there. Then the JVM tries to traverse the classloaders graph to ask them for the class but it can't do so since there are some classloader being garbage collected at the same time. So the JVM waits for for concurrent gc to finish and block the thread until that moment.

          For my application I'll apply the patch you've provided plus the modifications to the TomcatAnnotationLifecycleProvider.isAvailable so the server doesn't freeze.

          I understand this issue is not directly related to this bug. It couldn't happen before the patch because of the OOM, so this patch kind of unveal this issue

          Do you want me to open a new bug?. Maybe you could find a different way of detecting the presence of tomcat

          Show
          Ruben Martin Pozo added a comment - I've run the same test with patch2 and the problem keeps happening. The server freezes for 40 seconds in the method TomcatAnnotationLifecycleProvider.isAvailable. I run a little test. I changed that method to always return false since my application is running on a glassfish not a tomcat. And magically everything worked!!!. The classloaders are garbage collected and the server doesn't freeze. Here is what I think is happening. It might sound science fiction but here it goes: I'm running with the flag -XX:+UseConcMarkSweepGC so I'm running a concurrent garbage collector. When there isn't any more room in the permGen for classes, a permGen gc cycle is run. While running this cycle the method TomcatAnnotationLifecycleProvider.isAvailable is trying to know if tomcat is available. To do so, it tries to load a tomcat class. But I don't have this class in my classpath since I'm not running on a tomcat. The JVM looks for the class in the entire classpath but it doesn't find it there. Then the JVM tries to traverse the classloaders graph to ask them for the class but it can't do so since there are some classloader being garbage collected at the same time. So the JVM waits for for concurrent gc to finish and block the thread until that moment. For my application I'll apply the patch you've provided plus the modifications to the TomcatAnnotationLifecycleProvider.isAvailable so the server doesn't freeze. I understand this issue is not directly related to this bug. It couldn't happen before the patch because of the OOM, so this patch kind of unveal this issue Do you want me to open a new bug?. Maybe you could find a different way of detecting the presence of tomcat
          Hide
          Leonardo Uribe added a comment -

          Well, do you have a better explanation? You could try replace SoftReference with WeakReference. If that change improves the situation, let me know and I'll change the code.

          Show
          Leonardo Uribe added a comment - Well, do you have a better explanation? You could try replace SoftReference with WeakReference. If that change improves the situation, let me know and I'll change the code.
          Hide
          Ruben Martin Pozo added a comment -

          I'm sorry but I disagree. I've run the test 20 times and every single time the server gets stuck at the same point in the code. That's not justified by the gc but more likely by some collateral problem caused by the soft references being removed

          If I run the test with the previous version of MyFaces where the memory leak doesn't exist I don't see this behavior and the gc is also unloading classloaders and classes.

          Tomorrow I'll run the test with patch-2 and I'll let you know the results

          Show
          Ruben Martin Pozo added a comment - I'm sorry but I disagree. I've run the test 20 times and every single time the server gets stuck at the same point in the code. That's not justified by the gc but more likely by some collateral problem caused by the soft references being removed If I run the test with the previous version of MyFaces where the memory leak doesn't exist I don't see this behavior and the gc is also unloading classloaders and classes. Tomorrow I'll run the test with patch-2 and I'll let you know the results
          Hide
          Leonardo Uribe added a comment -

          Garbage collection takes time. The code committed (patch-2) try to cleanup the cache as soon as the app is undeployed. The soft reference is only cleaned up when memory is really necessary, and an OutOfMemoryError is only thrown after clean all soft references. I suppose the GC is responsible of the slowdown, but with the code committed, this effect dissapear.

          Show
          Leonardo Uribe added a comment - Garbage collection takes time. The code committed (patch-2) try to cleanup the cache as soon as the app is undeployed. The soft reference is only cleaned up when memory is really necessary, and an OutOfMemoryError is only thrown after clean all soft references. I suppose the GC is responsible of the slowdown, but with the code committed, this effect dissapear.
          Hide
          Ruben Martin Pozo added a comment - - edited

          And how do you explain that if I remove the patch the server doesn't freeze when deploying? To test the patch I have to redeploy the app several times so I get to the point when there is no more permGen memory and the soft reference is garbage collected. Then and only then the server freezes and not in any of the other times when I redeployed but there was enough memory

          So I really think something strange is happening that is, somehow, triggered by this patch.

          I'll try patch-2 to see if the same thing happens

          Show
          Ruben Martin Pozo added a comment - - edited And how do you explain that if I remove the patch the server doesn't freeze when deploying? To test the patch I have to redeploy the app several times so I get to the point when there is no more permGen memory and the soft reference is garbage collected. Then and only then the server freezes and not in any of the other times when I redeployed but there was enough memory So I really think something strange is happening that is, somehow, triggered by this patch. I'll try patch-2 to see if the same thing happens
          Hide
          Leonardo Uribe added a comment -

          The stack trace provided looks normal. A webapp is deployed, so it takes some time to initialize the environment. That is necessary because it is required to parse some files, scan for annotations and merge all that information. Please do not reopen this issue and instead, create another one if necessary, because the stack trace is another different behavior to the one solved in the patch.

          Show
          Leonardo Uribe added a comment - The stack trace provided looks normal. A webapp is deployed, so it takes some time to initialize the environment. That is necessary because it is required to parse some files, scan for annotations and merge all that information. Please do not reopen this issue and instead, create another one if necessary, because the stack trace is another different behavior to the one solved in the patch.
          Hide
          Ruben Martin Pozo added a comment -

          I've seen you uploaded a new patch while I was writing the comment

          I'll try the new patch and I'll let you know if this issue keeps happening

          Show
          Ruben Martin Pozo added a comment - I've seen you uploaded a new patch while I was writing the comment I'll try the new patch and I'll let you know if this issue keeps happening
          Hide
          Ruben Martin Pozo added a comment -

          There's an strange behaviour after applying the patch

          Show
          Ruben Martin Pozo added a comment - There's an strange behaviour after applying the patch
          Hide
          Ruben Martin Pozo added a comment - - edited

          I've tried the patch and it fixes the problem. The application classloader is gced before throwing the OOM

          But then something very very strange happens

          Just after collecting the classloader (I see that in the Java Visual VM tool) the CPU rises up to the 50% (1 core) and the application server freezes for 20-40 seconds. The only thread working is the one copied below. After that time the application displays the JSF page without errors.

          Any guess???

          2012-03-22 18:21:59

          "pool-1-thread-3" - Thread t@233
          java.lang.Thread.State: RUNNABLE
          at org.apache.myfaces.config.annotation.TomcatAnnotationLifecycleProvider.isAvailable(TomcatAnnotationLifecycleProvider.java:75)
          at org.apache.myfaces.config.annotation.DefaultLifecycleProviderFactory.resolveLifecycleProviderFromService(DefaultLifecycleProviderFactory.java:208)
          at org.apache.myfaces.config.annotation.DefaultLifecycleProviderFactory.getLifecycleProvider(DefaultLifecycleProviderFactory.java:86)
          at org.apache.myfaces.config.FacesConfigurator.configureManagedBeanDestroyer(FacesConfigurator.java:1043)
          at org.apache.myfaces.config.FacesConfigurator.configure(FacesConfigurator.java:431)
          at org.apache.myfaces.webapp.AbstractFacesInitializer.buildConfiguration(AbstractFacesInitializer.java:338)
          at org.apache.myfaces.webapp.Jsp21FacesInitializer.initContainerIntegration(Jsp21FacesInitializer.java:73)
          at org.apache.myfaces.webapp.AbstractFacesInitializer.initFaces(AbstractFacesInitializer.java:140)
          at org.apache.myfaces.webapp.StartupServletContextListener.contextInitialized(StartupServletContextListener.java:111)
          at org.apache.catalina.core.StandardContext.contextListenerStart(StandardContext.java:4690)
          at com.sun.enterprise.web.WebModule.contextListenerStart(WebModule.java:534)
          at org.apache.catalina.core.StandardContext.start(StandardContext.java:5305)

          • locked <f1a64> (a com.sun.enterprise.web.WebModule)
            at com.sun.enterprise.web.WebModule.start(WebModule.java:500)
          • locked <f1a64> (a com.sun.enterprise.web.WebModule)
            at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:917)
          • locked <aefd97> (a java.util.LinkedHashMap)
            at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:901)
            at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:755)
            at com.sun.enterprise.web.WebContainer.loadWebModule(WebContainer.java:1980)
            at com.sun.enterprise.web.WebContainer.loadWebModule(WebContainer.java:1630)
            at com.sun.enterprise.web.WebApplication.start(WebApplication.java:100)
            at org.glassfish.internal.data.EngineRef.start(EngineRef.java:130)
            at org.glassfish.internal.data.ModuleInfo.start(ModuleInfo.java:269)
          • locked <17b599c> (a org.glassfish.internal.data.ModuleInfo)
            at org.glassfish.internal.data.ApplicationInfo.start(ApplicationInfo.java:286)
            at com.sun.enterprise.v3.server.ApplicationLifecycle.deploy(ApplicationLifecycle.java:461)
            at com.sun.enterprise.v3.server.ApplicationLifecycle.deploy(ApplicationLifecycle.java:240)
            at org.glassfish.deployment.admin.DeployCommand.execute(DeployCommand.java:370)
            at com.sun.enterprise.v3.admin.CommandRunnerImpl$1.execute(CommandRunnerImpl.java:355)
            at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:370)
            at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:1067)
            at com.sun.enterprise.v3.admin.CommandRunnerImpl.access$1200(CommandRunnerImpl.java:96)
            at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1247)
            at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1235)
            at com.sun.enterprise.admin.cli.embeddable.CommandExecutorImpl.executeCommand(CommandExecutorImpl.java:118)
            at com.sun.enterprise.admin.cli.embeddable.DeployerImpl.deploy(DeployerImpl.java:97)
            at com.sun.enterprise.admin.cli.embeddable.DeployerImpl.deploy(DeployerImpl.java:88)
            at com.isb.runtime.embedded.remote.RuntimeRMIServerImpl.forceDeploy(RuntimeRMIServerImpl.java:364)
          • locked <8c436b> (a com.isb.runtime.embedded.remote.RuntimeRMIServerImpl)
            at com.isb.runtime.embedded.remote.RuntimeRMIServerImpl.requestAccepted(RuntimeRMIServerImpl.java:258)
          • locked <8c436b> (a com.isb.runtime.embedded.remote.RuntimeRMIServerImpl)
            at com.isb.runtime.embedded.remote.ServerDeployProxy.deployApplication(ServerDeployProxy.java:449)
            at com.isb.runtime.embedded.remote.ServerDeployProxy.access$0(ServerDeployProxy.java:447)
            at com.isb.runtime.embedded.remote.ServerDeployProxy$DeployTask.call(ServerDeployProxy.java:107)
            at com.isb.runtime.embedded.remote.ServerDeployProxy$DeployTask.call(ServerDeployProxy.java:1)
            at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
            at java.util.concurrent.FutureTask.run(FutureTask.java:138)
            at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
            at java.lang.Thread.run(Thread.java:662)

          Locked ownable synchronizers:

          • locked <11d95d> (a java.util.concurrent.locks.ReentrantLock$NonfairSync)
          Show
          Ruben Martin Pozo added a comment - - edited I've tried the patch and it fixes the problem. The application classloader is gced before throwing the OOM But then something very very strange happens Just after collecting the classloader (I see that in the Java Visual VM tool) the CPU rises up to the 50% (1 core) and the application server freezes for 20-40 seconds. The only thread working is the one copied below. After that time the application displays the JSF page without errors. Any guess??? 2012-03-22 18:21:59 "pool-1-thread-3" - Thread t@233 java.lang.Thread.State: RUNNABLE at org.apache.myfaces.config.annotation.TomcatAnnotationLifecycleProvider.isAvailable(TomcatAnnotationLifecycleProvider.java:75) at org.apache.myfaces.config.annotation.DefaultLifecycleProviderFactory.resolveLifecycleProviderFromService(DefaultLifecycleProviderFactory.java:208) at org.apache.myfaces.config.annotation.DefaultLifecycleProviderFactory.getLifecycleProvider(DefaultLifecycleProviderFactory.java:86) at org.apache.myfaces.config.FacesConfigurator.configureManagedBeanDestroyer(FacesConfigurator.java:1043) at org.apache.myfaces.config.FacesConfigurator.configure(FacesConfigurator.java:431) at org.apache.myfaces.webapp.AbstractFacesInitializer.buildConfiguration(AbstractFacesInitializer.java:338) at org.apache.myfaces.webapp.Jsp21FacesInitializer.initContainerIntegration(Jsp21FacesInitializer.java:73) at org.apache.myfaces.webapp.AbstractFacesInitializer.initFaces(AbstractFacesInitializer.java:140) at org.apache.myfaces.webapp.StartupServletContextListener.contextInitialized(StartupServletContextListener.java:111) at org.apache.catalina.core.StandardContext.contextListenerStart(StandardContext.java:4690) at com.sun.enterprise.web.WebModule.contextListenerStart(WebModule.java:534) at org.apache.catalina.core.StandardContext.start(StandardContext.java:5305) locked <f1a64> (a com.sun.enterprise.web.WebModule) at com.sun.enterprise.web.WebModule.start(WebModule.java:500) locked <f1a64> (a com.sun.enterprise.web.WebModule) at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:917) locked <aefd97> (a java.util.LinkedHashMap) at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:901) at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:755) at com.sun.enterprise.web.WebContainer.loadWebModule(WebContainer.java:1980) at com.sun.enterprise.web.WebContainer.loadWebModule(WebContainer.java:1630) at com.sun.enterprise.web.WebApplication.start(WebApplication.java:100) at org.glassfish.internal.data.EngineRef.start(EngineRef.java:130) at org.glassfish.internal.data.ModuleInfo.start(ModuleInfo.java:269) locked <17b599c> (a org.glassfish.internal.data.ModuleInfo) at org.glassfish.internal.data.ApplicationInfo.start(ApplicationInfo.java:286) at com.sun.enterprise.v3.server.ApplicationLifecycle.deploy(ApplicationLifecycle.java:461) at com.sun.enterprise.v3.server.ApplicationLifecycle.deploy(ApplicationLifecycle.java:240) at org.glassfish.deployment.admin.DeployCommand.execute(DeployCommand.java:370) at com.sun.enterprise.v3.admin.CommandRunnerImpl$1.execute(CommandRunnerImpl.java:355) at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:370) at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:1067) at com.sun.enterprise.v3.admin.CommandRunnerImpl.access$1200(CommandRunnerImpl.java:96) at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1247) at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1235) at com.sun.enterprise.admin.cli.embeddable.CommandExecutorImpl.executeCommand(CommandExecutorImpl.java:118) at com.sun.enterprise.admin.cli.embeddable.DeployerImpl.deploy(DeployerImpl.java:97) at com.sun.enterprise.admin.cli.embeddable.DeployerImpl.deploy(DeployerImpl.java:88) at com.isb.runtime.embedded.remote.RuntimeRMIServerImpl.forceDeploy(RuntimeRMIServerImpl.java:364) locked <8c436b> (a com.isb.runtime.embedded.remote.RuntimeRMIServerImpl) at com.isb.runtime.embedded.remote.RuntimeRMIServerImpl.requestAccepted(RuntimeRMIServerImpl.java:258) locked <8c436b> (a com.isb.runtime.embedded.remote.RuntimeRMIServerImpl) at com.isb.runtime.embedded.remote.ServerDeployProxy.deployApplication(ServerDeployProxy.java:449) at com.isb.runtime.embedded.remote.ServerDeployProxy.access$0(ServerDeployProxy.java:447) at com.isb.runtime.embedded.remote.ServerDeployProxy$DeployTask.call(ServerDeployProxy.java:107) at com.isb.runtime.embedded.remote.ServerDeployProxy$DeployTask.call(ServerDeployProxy.java:1) at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303) at java.util.concurrent.FutureTask.run(FutureTask.java:138) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908) at java.lang.Thread.run(Thread.java:662) Locked ownable synchronizers: locked <11d95d> (a java.util.concurrent.locks.ReentrantLock$NonfairSync)
          Hide
          Leonardo Uribe added a comment -

          I have attached another patch that uses this structure:

          private static Map<ClassLoader, SoftReference<Map<Class<?>, Map<String, _PropertyDescriptorHolder>>>>
          propertyDescriptorCacheMap = new WeakHashMap<ClassLoader, SoftReference<Map<Class<?>,
          Map<String, _PropertyDescriptorHolder>>>>();

          The idea is use a SoftReference to prevent the direct strong reference over propertyDescriptorCache, but use a new key over current classloader to make possible when the application is undeployed to clean the cache.

          Using yourkit profiler I was not able to see any problem with "metadata" field, because a cleanup method is called when the application is destroyed on AbstractFacesInitializer.

          I also tried to check if the SoftReference solves the problem without the cleanup code and it does. I also checked if the solution makes the code slower, but the effect is minimal.

          Show
          Leonardo Uribe added a comment - I have attached another patch that uses this structure: private static Map<ClassLoader, SoftReference<Map<Class<?>, Map<String, _PropertyDescriptorHolder>>>> propertyDescriptorCacheMap = new WeakHashMap<ClassLoader, SoftReference<Map<Class<?>, Map<String, _PropertyDescriptorHolder>>>>(); The idea is use a SoftReference to prevent the direct strong reference over propertyDescriptorCache, but use a new key over current classloader to make possible when the application is undeployed to clean the cache. Using yourkit profiler I was not able to see any problem with "metadata" field, because a cleanup method is called when the application is destroyed on AbstractFacesInitializer. I also tried to check if the SoftReference solves the problem without the cleanup code and it does. I also checked if the solution makes the code slower, but the effect is minimal.
          Hide
          Leonardo Uribe added a comment -

          I have attached a patch with a possible solution. The idea is change propertyDescriptorCache, adding a soft reference.

          private static Map<Class<?>, SoftReference<Map<String, _PropertyDescriptorHolder>>>
          propertyDescriptorCache =
          new WeakHashMap<Class<?>, SoftReference<Map<String, _PropertyDescriptorHolder>>>();

          In this way, we can avoid the strong reference through the static var, and since the garbage collector needs to clean all soft references before throw an OutOfMemoryError, we ensure that cache is cleaned up if memory is required.

          @Ruben: Could you please try if this patch can solve the problem?

          Show
          Leonardo Uribe added a comment - I have attached a patch with a possible solution. The idea is change propertyDescriptorCache, adding a soft reference. private static Map<Class<?>, SoftReference<Map<String, _PropertyDescriptorHolder>>> propertyDescriptorCache = new WeakHashMap<Class<?>, SoftReference<Map<String, _PropertyDescriptorHolder>>>(); In this way, we can avoid the strong reference through the static var, and since the garbage collector needs to clean all soft references before throw an OutOfMemoryError, we ensure that cache is cleaned up if memory is required. @Ruben: Could you please try if this patch can solve the problem?
          Hide
          Leonardo Uribe added a comment -

          What I mean with "weakly reachable" is since the map in propertyDescriptorCache is a WeakHashMap, once the keys are collected, the values are discarded and the Classloader can be discarded from memory too. So, "strictly speaking" there is no leak. What could happen is in one moment of the time the Classloader cannot be discarded since the weak keys needs to be discarded first.

          Could you see if there is a leak over org.apache.myfaces.view.facelets.tag.MetaRulesetImpl (used in facelets)?, specifically with:

          private volatile static WeakHashMap<ClassLoader, Map<String, MetadataTarget>> metadata
          = new WeakHashMap<ClassLoader, Map<String, MetadataTarget>>();

          This is a similar case. If this is not a problem, maybe the leak is caused by cache Method instances.

          Show
          Leonardo Uribe added a comment - What I mean with "weakly reachable" is since the map in propertyDescriptorCache is a WeakHashMap, once the keys are collected, the values are discarded and the Classloader can be discarded from memory too. So, "strictly speaking" there is no leak. What could happen is in one moment of the time the Classloader cannot be discarded since the weak keys needs to be discarded first. Could you see if there is a leak over org.apache.myfaces.view.facelets.tag.MetaRulesetImpl (used in facelets)?, specifically with: private volatile static WeakHashMap<ClassLoader, Map<String, MetadataTarget>> metadata = new WeakHashMap<ClassLoader, Map<String, MetadataTarget>>(); This is a similar case. If this is not a problem, maybe the leak is caused by cache Method instances.
          Hide
          Ruben Martin Pozo added a comment -

          This is a fragment of the WeakHashMap documentation

          "Implementation note: The value objects in a WeakHashMap are held by ordinary strong references. Thus care should be taken to ensure that value objects do not strongly refer to their own keys, either directly or indirectly, since that will prevent the keys from being discarded. Note that a value object may refer indirectly to its key via the WeakHashMap itself; that is, a value object may strongly refer to some other key object whose associated value object, in turn, strongly refers to the key of the first value object. One way to deal with this is to wrap values themselves within WeakReferences before inserting, as in: m.put(key, new WeakReference(value)), and then unwrapping upon each get."

          Show
          Ruben Martin Pozo added a comment - This is a fragment of the WeakHashMap documentation "Implementation note: The value objects in a WeakHashMap are held by ordinary strong references. Thus care should be taken to ensure that value objects do not strongly refer to their own keys, either directly or indirectly, since that will prevent the keys from being discarded. Note that a value object may refer indirectly to its key via the WeakHashMap itself; that is, a value object may strongly refer to some other key object whose associated value object, in turn, strongly refers to the key of the first value object. One way to deal with this is to wrap values themselves within WeakReferences before inserting, as in: m.put(key, new WeakReference(value)), and then unwrapping upon each get."
          Hide
          Ruben Martin Pozo added a comment -

          What do you mean when you say that the classloader is weakly reachable? JProfiler is only showing strong references. If you mean its weakly reachable because of the WeakHashMap that's not correct since WeakHashMap only uses weakreference for keys not for values as this is the case.

          I'm getting OOM because we're running out of permGem memory due to the application class loader leak

          Show
          Ruben Martin Pozo added a comment - What do you mean when you say that the classloader is weakly reachable? JProfiler is only showing strong references. If you mean its weakly reachable because of the WeakHashMap that's not correct since WeakHashMap only uses weakreference for keys not for values as this is the case. I'm getting OOM because we're running out of permGem memory due to the application class loader leak
          Hide
          Leonardo Uribe added a comment - - edited

          I checked the code and it works as it was designed. There is a WeakHashMap in _ComponentAttributesMap:

          // Cache for component property descriptors
          private static Map<Class<?>, Map<String, _PropertyDescriptorHolder>> propertyDescriptorCache =
          new WeakHashMap<Class<?>, Map<String, _PropertyDescriptorHolder>>();

          and _PropertyDescriptorHolder holds PropertyDescriptor and Method instances. It is not necessary to use WeakReference inside _PropertyDescriptorHolder, because the Classloader is already weakly reachable.

          I took an application using tomcat 7.0.23 and then try to start/stop multiple times. If there is a memory leak, an OutOfMemoryException can be seen. It never happened.

          What could happen is the classloader is not discarded by the gc immediately. But as soons as the jvm ask for memory and the gc do its cleanup job, it will found the classloader is weakly reachable and can be discarded from memory.

          Unfortunately, the screenshot is not enough proof that a memory leak exist. We need to check if an OutOfMemoryException could be caused by this code.

          Anyway, use a WeakReference is only reasonable if you can gain something, but I don't think it apply in this case.

          Suggestions are welcome.

          Show
          Leonardo Uribe added a comment - - edited I checked the code and it works as it was designed. There is a WeakHashMap in _ComponentAttributesMap: // Cache for component property descriptors private static Map<Class<?>, Map<String, _PropertyDescriptorHolder>> propertyDescriptorCache = new WeakHashMap<Class<?>, Map<String, _PropertyDescriptorHolder>>(); and _PropertyDescriptorHolder holds PropertyDescriptor and Method instances. It is not necessary to use WeakReference inside _PropertyDescriptorHolder, because the Classloader is already weakly reachable. I took an application using tomcat 7.0.23 and then try to start/stop multiple times. If there is a memory leak, an OutOfMemoryException can be seen. It never happened. What could happen is the classloader is not discarded by the gc immediately. But as soons as the jvm ask for memory and the gc do its cleanup job, it will found the classloader is weakly reachable and can be discarded from memory. Unfortunately, the screenshot is not enough proof that a memory leak exist. We need to check if an OutOfMemoryException could be caused by this code. Anyway, use a WeakReference is only reasonable if you can gain something, but I don't think it apply in this case. Suggestions are welcome.
          Hide
          Jesús Pérez Alcaide (ISBAN) added a comment -

          This seems to be related to MYFACES-3262.

          Show
          Jesús Pérez Alcaide (ISBAN) added a comment - This seems to be related to MYFACES-3262 .
          Hide
          Ruben Martin Pozo added a comment -

          Here you can see the problem as shown in jprofiler

          Show
          Ruben Martin Pozo added a comment - Here you can see the problem as shown in jprofiler

            People

            • Assignee:
              Leonardo Uribe
              Reporter:
              Ruben Martin Pozo
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development