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

Paul Sandoz psandoz at openjdk.java.net
Thu Aug 26 18:16:29 UTC 2021


On Wed, 25 Aug 2021 09:31:51 GMT, Vladimir Ivanov <vlivanov 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

Looks good.

I guess it is not that common for the soft ref to get instantiated i.e. for the case of the ~common class loader of `type`, MTC say, and the ~common classoader of `newType`, NMTC say, then NMTC is not an ancestor of MTC.

It's possible that `asTypeCache` and `asTypeSoftCache` could both be non-null i.e. we don't null out one of them, buy does not seem a problem.

src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 869:

> 867:         }
> 868:         at = asTypeUncached(newType);
> 869:         return setAsTypeCache(at);

The following may be a little clearer
Suggestion:

        MethodHandle at = asTypeCached(newType);
        return at != null
            ? at
            : setAsTypeCache(asTypeUncached(newType));

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.

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

Marked as reviewed by psandoz (Reviewer).

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


More information about the core-libs-dev mailing list