RFR: 8304147: JVM crash during shutdown when dumping dynamic archive [v2]

[David Holmes]

On Fri, 24 Mar 2023 18:38:43 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> David Holmes has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision:
>> 
>>  - Restore comment to as it was pre JDK-8266770
>>  - Update test to removing debugging code
>>  - Merge branch 'master' into 8304147-dynamic-dump
>>  - Update test comments
>>  - Phase 4: remove unused methods and adjust dump_for_jcmd
>>  - Missed removing the check at the dump_at_exit callsite.
>>  - Phase 3: Consolidate code into a new dump_at_exit that combines check, prepare and dump in one.
>>    Minor tweak to test.
>>  - Merge branch '8304147-test-only' into 8304147-dynamic-dump
>>  - Initial test
>>  - Phase 2: Move the dump to immediately after the prepare_for_dump
>>  - ... and 2 more: https://git.openjdk.org/jdk/compare/955564c9...518f5409
>
> src/hotspot/share/cds/dynamicArchive.cpp line 385:
> 
>> 383:   HandleMark hm(current);
>> 384: 
>> 385:   if (!(DynamicDumpSharedSpaces && archive_name != nullptr)) {
> 
> Can you make this !DynamicDumpSharedSpaces || archive_name == nullptr so it's easier to read.  I'm trying to figure out how archive_name relates to ArchiveClassesAtExit and why you made the switch.

I changed the condition to use a temp variable that should make things clearer.

`ArchiveClassesAtExit` was always passed to `dump` as the `archive_name`, so no "switch" there.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13134#discussion_r1148781382