[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