[jboss-jira] [JBoss JIRA] Updated: (JASSIST-104) Proxy cache fails to guard against circular references which inhibits GC of classloaders

[Andrew Dinn (JIRA)]

     [ https://jira.jboss.org/jira/browse/JASSIST-104?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrew Dinn updated JASSIST-104:
--------------------------------

    Attachment: jassist-104-patch.diffs


This patches the proxy cache so that details of a cached proxy -- super class, handler and filter -- are only retained via weak references.

The enclosed test uses javassist to create a super, filter and handler class local to a newly created class loader. It then proceeds to create a proxy factory, class and instance using these three objects. All  references to these classes/objects (inlcuding the classloader) are then dropped.

The test repeats this operation 10000 times without incurring a perm gen out of memory exception. If instead the superclass is changed to be java.lang.Object the test gets a perm-gen error because the proxy and its factory are associated with Object's classloader and so they never get released.

Obviously the number of repetitions required to generate a failure when the superclass type is switched will vary depending upon the JVM heap size. So, this is merely a smoke test rather than a guarantee of correct performance.


> Proxy cache fails to guard against circular references which inhibits GC of classloaders
> ----------------------------------------------------------------------------------------
>
>                 Key: JASSIST-104
>                 URL: https://jira.jboss.org/jira/browse/JASSIST-104
>             Project: Javassist
>          Issue Type: Bug
>    Affects Versions: 3.11.0.GA
>            Reporter: Andrew Dinn
>            Assignee: Andrew Dinn
>         Attachments: jassist-104-patch.diffs
>
>
> The proxy cache employs a two level map to retain details of created proxies. The primary map  is a WeakHashMap which associates classloaders with all proxies created using that classloader. The WeakHashMap is intended to ensure that references to proxies are dropped when a classloader is unloaded.
> The secondary maps appearing as values in the primary map are just  plain hashmaps. These are used to lookup details of a proxy using a name  constructed from the names of its superclass and the interfaces it implements. The target of the secondary map is a CacheKey object which stores  the munged name, a weak reference to the proxy class, the method filter and the current default handler for instances of the proxy class. Any such proxy class will, of  course,  reference its classloader, the very same loader used as the WeakHashMap key for the secondary map in which the CacheKey instance resides. A WeakReference to the proxy class breask the reference cycle, ensuring that the WeakHashMap entry can be cleared when all other references to the classloader have been dropped.
> Unfortunately, this is not enough. Cachekey contains two other fields, handler and filter, which are instances of interfaces MethodHandler and MethodFilter. The values in these fields are supplied by a client application when the proxy is created. In normal use the proxy factory will locate the proxy class in the client application's classloader. In most cases this will be the same classloader that defines the classes used to provide the implementations of MethodHandler and MethodFilter used to populate the handler and filter fields. So, the secondary map must not  employ a direct reference to the objects in these fields because this may result in a cyclic reference to the classloader and thereby inhibit GC.
> There is actually an extra detail in play here which affects any proposed solution. The secondary map key is not actually the munged name. Instead the CacheKey instance is used as both key and value. This is because the equals method for CacheKey checks that the munged name, filter and handler are all equal. Incorporating the filter and handler into the equality check allows caching of proxies for  the same class using different filters and handlers.
> Unfortunately, this is unsafe. The handler field is meant to be updateable by the client after creation of the proxy i.e. after entering it in the cache. Changing this without removing and reinserting the entry risks the integrity of subsequent hash table lookup, replace and remove operations. For example, it is quite easy to end up with two 'equal' entries in the hashtable.
> A solution would be to employ a WeakHashMap for each secondary map, use the munged name as key and employ a class similar to the current CacheKey as the target of the map with the difference that it contains a list of proxy/handler/filter triples in place of the current 3 individual references. This simplifies the lookup (no special equals or hashcode needed) and also minimises the number of weak references (1 per super/interface set). This will also simplify duplicate detection and elimination when the handler  is updated.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira