RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading

Mandy Chung mchung at openjdk.java.net
Mon Aug 30 23:58:32 UTC 2021


On Thu, 26 Aug 2021 17:57:26 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> `MethodHandle.asTypeCache` keeps a strong reference to adapted `MethodHandle` and it can introduce a class loader leak through its `MethodType`.
>> 
>> Proposed fix introduces a 2-level cache (1 element each) where 1st level can only contain `MethodHandle`s which are guaranteed to not introduce any dependencies on new class loaders compared to the original `MethodHandle`. 2nd level is backed by a `SoftReference` and is used as a backup when the result of `MethodHandle.asType()` conversion can't populate the higher level cache.  
>> 
>> The fix is based on [the work](http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/) made by Peter Levart @plevart back in 2015.
>> 
>> Testing: tier1 - tier6
>
> src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 953:
> 
>> 951: 
>> 952:     /* Determine whether {@code descendant} keeps {@code ancestor} alive through the loader delegation chain. */
>> 953:     private static boolean keepsAlive(ClassLoader ancestor, ClassLoader descendant) {
> 
> Might be clearer to name the method by what it is e.g. isAncestor
>     // Returns true if ancestor can be found descendant's delegation chain.

This method is not exactly doing `isAncestor` check.  It returns true if `ancestor` is a builtin loader even it's not an ancestor of `descendent`.   I agree that it would be helpful if the method is named by what it is.

Maybe naming it `isAncestor` but move the `isSystemLoader(ancestor)` check out to  the caller.  Just a thought.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5246


More information about the core-libs-dev mailing list