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

[Kim Barrett]

On Tue, 6 Jun 2023 13:46:37 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/runtime/threads.cpp line 572:
>> 
>>> 570:     if (!main_thread->has_pending_exception()) {
>>> 571:       main_thread->smr_delete();
>>> 572:     }
>> 
>> I'm sorry for belaboring this change but since jni is the only caller of this and conditionally calls vm_exit_during_initialization() with the pending exception, why not just do it here and take out the code in jni?   I was confused for a while about the thread being null in the jni caller.
>
> Also init_globals2 should have a better name, like init_objects_and_globals() or something like that.  But that's not this issue.

I was later wondering the same thing. Here's my rationale for putting it here
instead of in the JNI code (i.e. agreeing with Coleen's suggestion).

I think neither place is really correct. Failure to create and initialize the
VM ought to always propogate out to the JNI client to decide what to do about
the failure. But we don't do that, not only here but all over the place. These
(and many other) failures are treated as grounds for terminating the process
containing the VM, regardless of what else that process might be doing. This
limits the utility of the VM in ways similar to not being able to have
multiple VMs (at the same time or serially) in one process. Oh well.

Given that, putting the vm_exit_during_initialization() in create_vm seems
simpler.  It's not even clear create_vm should ever return failure with
canTryAgain false; maybe it should always terminate if there's a non-retryable
failure.  But perhaps that's for a later day.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14238#discussion_r1219837835