On Thu, 26 Sep 2024 00:45:48 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
>> src/java.base/share/classes/jdk/internal/loader/BuiltinClassLoader.java line
>> 1092:
>>
>>> 1090: void resetArchivedStatesForAppClassLoader() {
>>> 1091: setClassPath(null);
>>> 1092: if (!moduleToReader.isEmpty()) moduleToReader.clear();
>>
>> Suggestion:
>>
>> if (!moduleToReader.isEmpty()) {
>> moduleToReader.clear();
>> }
>>
>>
>> Also, do we need to do the same thing for the platform loader as well?
>
> Added braces.
> The `setClassPath(null)` used to be in `ClassLoaders.AppClassLoader`. Based
> on investigations so far, the clearing of the `moduleToReader` map is
> required only for `AppClassLoader`.
Why does it need to clear `moduleToReader` only for app loader and not for
platform loader? Is it because the `moduleToReader` for the app loader may
contain reference to jar files that indirectly references some file system
objects?
Since moduleToReader is just a cache, I think it's better to always clear it
for both loaders. Also, the logic can be moved into BuiltinClassLoader:
class BuiltinClassLoader {
....
private void resetArchivedStates() {
ucp = null;
resourceCache = null;
setClassPath(null); // AppClassLoader will initialize this again
at runtime.
moduleToReader.clear();
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21048#discussion_r1779903867
