On Thu, 26 Aug 2021 17:57:26 GMT, Paul Sandoz <psan...@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