[jira] [Issue Comment Edited] (MYFACES-3510) Application components classloader memory leak

["Leonardo Uribe (Issue Comment Edited) (JIRA)" <de...@myfaces.apache.org>]

    [ https://issues.apache.org/jira/browse/MYFACES-3510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13235068#comment-13235068 ] 

Leonardo Uribe edited comment on MYFACES-3510 at 3/21/12 9:49 PM:
------------------------------------------------------------------

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.
                
      was (Author: lu4242):
    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 as 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.
                  
> Application components classloader memory leak
> ----------------------------------------------
>
>                 Key: MYFACES-3510
>                 URL: https://issues.apache.org/jira/browse/MYFACES-3510
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: General
>    Affects Versions: 2.1.6
>         Environment: WIndows Glassfish Embedded
>            Reporter: Ruben Martin Pozo
>         Attachments: screenshot-1.jpg
>
>
> 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

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira