On Wed, 1 Sep 2021 17:28:37 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> 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. Sounds good. Incorporated your suggestions along with some minor refactorings. ------------- PR: https://git.openjdk.java.net/jdk/pull/5246