[crac] RFR: 8349365: [CRaC] Handle javaagent adding jars to boostrap/system classpath
Timofei Pushkin
tpushkin at openjdk.org
Wed Feb 5 15:38:27 UTC 2025
On Wed, 5 Feb 2025 11:17:07 GMT, Radim Vansa <rvansa at openjdk.org> wrote:
> File descriptors owned by `ClassPathEntry` in `ClassLoader` are ignored during checkpoint.
LGTM, some minor comments. Also looks like it supersedes #162, maybe we should mention the author of that as a contributor?
src/hotspot/os/linux/crac_linux.cpp line 305:
> 303: bool ok = true;
> 304:
> 305: GrowableArray<int> boot_classpath_fds = ClassLoader::get_classpath_entry_fds();
I believe `GrowableArray` allocates in the resource area by default so a `ResourceMark` should probably be added?
src/hotspot/share/classfile/classLoader.cpp line 823:
> 821:
> 822: GrowableArray<int> ClassLoader::get_classpath_entry_fds() {
> 823: log_info(crac)("Listing classpath FDS");
I believe this should be debug, or even trace, or maybe you just forgot to remove this line. If it remains, "FDs" instead of "FDS" would be better.
src/hotspot/share/classfile/classLoader.hpp line 417:
> 415:
> 416: // returns list of file descriptors used for both boot and app classpath entries
> 417: static GrowableArray<int> get_classpath_entry_fds();
I am not sure about the name, I assume it's not `get_classpath_append_entry_fds()` because it also includes `_app_classpath_entries` but when CDS is not included neither are the app entries... And as I understand you are not sure we need these app entries for the issue at all (I am also not sure, would need to get a deeper understanding of where do all other checked FDs come from).
-------------
PR Review: https://git.openjdk.org/crac/pull/201#pullrequestreview-2595745043
PR Review Comment: https://git.openjdk.org/crac/pull/201#discussion_r1943184656
PR Review Comment: https://git.openjdk.org/crac/pull/201#discussion_r1942966687
PR Review Comment: https://git.openjdk.org/crac/pull/201#discussion_r1942923409
More information about the crac-dev
mailing list