RFR: 8308764: Reporting errors from create_vm may crash [v2]

[Kim Barrett]

On Fri, 2 Jun 2023 02:20:16 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> There are more details in the bug report but the basic fix is to not delete the main_thread so that it is available to report the exception that caused VM initialization to fail. Added some additional comments too.
>> 
>> Testing:
>>  - basic fault injection to test non-exception exit paths both before and after Universe:;is_fully_initialized() which exercises the failing use of JavaThread::current()
>>  -  tiers 1-3 sanity testing
>> 
>> Thanks.
>
> 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 seven additional commits since the last revision:
> 
>  - Changes per Coleen's suggestion.
>  - Changes per Coleen's suggestion.
>  - Merge branch 'master' into 8308764-create_vm_errors
>  - Fix indentation
>  - Merge branch 'master' into 8308764-create_vm_errors
>  - Fixed typo
>  - 8308764: Reporting errors from create_vm may crash

Changes requested by kbarrett (Reviewer).

src/hotspot/share/runtime/threads.cpp line 553:

> 551:     if (!main_thread->has_pending_exception()) {
> 552:       main_thread->smr_delete();
> 553:     }

We have the exact same problem below after the call to init_globals2 (very recently introduced by JDK-8240774).

src/hotspot/share/runtime/threads.cpp line 554:

> 552:       main_thread->smr_delete();
> 553:     }
> 554:     // Else don't delete main_thread as we need to report exceptions in the caller.

Is it really okay to just leak main_thread?  If the process is going to die anyway then sure, but I'm not sure
that's true for all potential callers.  Of course, there's probably lots of other stuff getting leaked, so maybe
it doesn't matter.

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

PR Review: https://git.openjdk.org/jdk/pull/14238#pullrequestreview-1459234438
PR Review Comment: https://git.openjdk.org/jdk/pull/14238#discussion_r1215447662
PR Review Comment: https://git.openjdk.org/jdk/pull/14238#discussion_r1215447508