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

[David Holmes]

On Fri, 24 Mar 2023 18:39:41 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/e79dce26...518f5409
>
> src/hotspot/share/cds/dynamicArchive.cpp line 383:
> 
>> 381:   ExceptionMark em(current);
>> 382:   ResourceMark rm(current);
>> 383:   HandleMark hm(current);
> 
> Why is there a HandleMark here?  Should it be lower in the call stack?

It is needed before calling `MetaspaceShared::link_shared_classes`. It seemed neatest to place all the "marks" together at the start of the method.

> src/hotspot/share/cds/dynamicArchive.cpp line 391:
> 
>> 389:   log_info(cds,dynamic)("Preparing for dynamic dump at exit in thread %s", current->name());
>> 390: 
>> 391:   MetaspaceShared::link_shared_classes(false/*not from jcmd*/, current);
> 
> link_shared_classes is declared with TRAPS, so this should have a
> JavaThread* THREAD = current;
> and use HAS_PENDING_EXCEPTION, CLEAR_PENDING_EXCEPTION macros.

Okay

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

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