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