RFR: 8078641: MethodHandle.asTypeCache can retain classes from unloading [v2]

Mandy Chung mchung at openjdk.java.net
Wed Sep 1 17:31:12 UTC 2021

On Wed, 1 Sep 2021 16:23:46 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
> Vladimir Ivanov has updated the pull request incrementally with one additional commit since the last revision:
>   Address review comments

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

> 924:     /* Returns true when {@code loader} keeps {@code cls} either directly or indirectly through the loader delegation chain. */
> 925:     private static boolean keepsAlive(Class<?> cls, ClassLoader loader) {
> 926:         return keepsAlive(cls.getClassLoader(), loader);


        ClassLoader defLoader = cls.getClassLoader();
        if (isBuiltinLoader(defLoader)) {
            return true; // built-in loaders are always reachable
        return keepsAlive(defLoader, loader);

I think it's clearer to check if `cls` is not defined by any builtin loader here and then check if `loader` keeps `cls` alive.

So `keepsAlive(ClassLoader loader1, ClassLoader loader2)` is not needed and replace line 935 and 940 to `keepsAlive(Class, ClassLoader)` instead.


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

More information about the core-libs-dev mailing list