On Wed, 1 Sep 2021 17:28:37 GMT, Mandy Chung <[email protected]> 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
