On Wed, 1 Sep 2021 16:23:46 GMT, Vladimir Ivanov <vliva...@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); Suggestion: 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
