[jira] Commented: (AXIS-2232) Mappings in TypeDesc can't be GC'ed

["Kevan Miller (JIRA)" <ax...@ws.apache.org>]

    [ http://issues.apache.org/jira/browse/AXIS-2232?page=comments#action_12356744 ] 

Kevan Miller commented on AXIS-2232:
------------------------------------

TypeDesc is holding on to Geronimo ClassLoaders. Although this patch is supposed to fix this problem, I'm afraid it isn't working... 

The patch supplied a static "classMaps" WeakHashMap. Each entry in this map has a ClassLoader key and a "classMap" Hashtable as the value. Each entry in the classMap Hashtable has a Class as key and a generated TypeDesc as the value. Since Class.loader is a strong reference to the ClassLoader, the "value" of the WeakHashMap will always contain a strong reference to the "key". So, as long as the TypeDesc Class object is alive, the ClassLoader key will never be GC'ed.

The chain of strong references goes something like: 

Parent ClassLoader -> TypeDesc class -> TypeDesc.classMaps -> WeakHashMap entry.value (strong reference) -> Hashtable -> HashTable entry.key  -> Class -> Class.loader -> Child ClassLoader

The fact that classMaps is a WeakHashMap is irrelevant... The ClassLoader will be kept alive by the above chain of strong references...

I originally fixed this by extending the current implementation. "classMap" becomes a WeakHashMap and TypeDesc.javaClass becomes a WeakReference to the Class object. There's a problem with this however. TypeDesc performs lazy (on-demand) processing to compute the BeanPropertyDescriptors for a Class. With a truly-weak TypeDesc cache, the Class referenced by TypeDesc could be GC'ed. So, it's possible that the Class could be GC'ed before getPropertyDescriptors() was called on TypeDesc (which is the reason it contains a javaClass reference, to begin with). The Class referenced by the javaClass weakReference could be GC'ed at anytime. The following code might not properly retrieve the BeanPropertyDescriptors for an object:

    TypeDesc desc = TypeDesc.getTypeDescForClass(someClassObject);
     someClassObject = null;
     desc.getPropertyDescriptors();  // Could fail because the Class has been GC'ed

There are multiple ways to fix. However, I don't really know what the common TypeDesc usage patterns are. So, I'm not sure of the most appropriate fix... Here are some possibilities:

1. Fix as described above -- have a truly weak classMap cache. I actually don't see a reason for a two-level cache. A single-level WeakHashMap keyed by Class would be sufficient. Invokers must be sure that a strong reference to the Class or it's ClassLoader is held from TypeDesc construction to PropertyDescriptor retrieval -- otherwise TypeDesc might lose its WeakReference to the Class...
2. Remove the static caching function from the TypeDesc class. TypeDesc objects can strongly reference the Class. TypeDesc object instances (not the Class instance) may keep the Classes/ClassLoader alive, but once the TypeDesc object is eligible for garbage collection, so are the Classes/ClassLoader. I don't know what kind of performance impacts the removal of caching support might have.
3. Aggressively compute the BeanPropertyDescriptors for a Class when a TypeDesc is constructed (rather than waiting until the descriptors are requested). This means the TypeDesc does not need to reference the Class or ClassLoader. TypeDesc instances can be safely cached using a WeakHashMap. I don't know what kind of performance impact this might have. How frequently is a TypeDesc constructed without the PropertyDescriptors being retrieved?
4. Add a TypeDesc.flushCache(ClassLoader) method and require flushCache() to be called as appropriate (i.e. when ClassLoaders are going out of scope...). This would work, but I hate to add this interdependency...

Comments? It's likely that 1 works in current usage scenarios. I guess 3 is perhaps the safest and is my favorite. However, I don't know what your usage patterns are... Let me know if you have an opinion -- I'm happy to create a patch...

> Mappings in TypeDesc can't be GC'ed
> -----------------------------------
>
>          Key: AXIS-2232
>          URL: http://issues.apache.org/jira/browse/AXIS-2232
>      Project: Apache Axis
>         Type: Improvement
>   Components: Deployment / Registries
>     Versions: current (nightly)
>     Reporter: David Blevins
>  Attachments: TypeDesc.java.patch
>
> The TypeDesc class holds a static Hashtable of class -> TypeDesc mappings.  This is fine if Axis is loaded into the only the same classloader as the app itself (like when it's embedded in a webapp), but if axis is loaded into the parent classloader of all webapps, the hashtable will prevent those children classloaders from being garbage collected as it holds references to classes in those children classloaders.
> This patch 1) creates a Hashtable of class->TypeDesc for *each classloader* and 2) stores those hashtables in a WeakHashMap so they can be garbage collected when the child classloaders are garbage collected.

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