[
https://jira.jboss.org/jira/browse/JASSIST-104?page=com.atlassian.jira.pl...
]
Andrew Dinn commented on JASSIST-104:
-------------------------------------
There is another problem here which my test has uncovered which makes me suspicious that
there is another related issue here.
My test fails on the unpatched code and works with my patch to ProxyFactory. The patched
version also fails if I provide Object as the super class which means that all the proxies
are cached under the system class loader.
However, I also ran the test using Object as the super class and with
ProxyFactory.useCache=false and it still crashed. So, it is not the proxy cache data which
is causing the problem but the retention of the generated proxy classes themselves.
Actually, this is obvious from the error. The cache data is not allocated out of perm gen
space but creation of the classes does consume perm gen space.
I reran the test with a tiny heap (-Xms10M -Xmx10M) and it still worked for 5000
repetitions and failed for 10000. So, I am pretty certain the out of space error is to do
with retaining handles on the proxy classes and nothing to do with retaining the cache
data..That's not a lot of classes. Is there something really inefficient going on
here? Or is it just very expensive to create 10000 classes at runtme?
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