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

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.