[foreign-memaccess+abi] RFR: 8270851: Logic for attaching/detaching native threads could be improved [v2]

[David Holmes]

On Tue, 29 Mar 2022 09:11:36 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> src/hotspot/share/prims/universalUpcallHandler.cpp line 67:
>> 
>>> 65:     JNIEnv* p_env = nullptr; // unused
>>> 66:     jint result = vm->functions->AttachCurrentThread(vm, (void**) &p_env, nullptr);
>>> 67:     guarantee(result == JNI_OK, "Could not attach thread for upcall. JNI error code: %d", result);
>> 
>> Do you really want to abort if something goes wrong i.e. out-of-memory**? (Also the JNI error code won't be very illuminating as only JNI_ERR is ever returned from AttachCurrentThread.)
>> 
>> ** Any unexpected exceptions from the java.lang.Thread construction will be silently cleared but also cause failure.
>
> For errors that occurs inside the upcall, we generally try to print a stack trace - but in this case we're a bit of a limbo between native and Java - and we still don't have a Java thread - so I'm assuming that if something goes wrong here there's not much we can do to dress it up nicely and/or recover? I agree that printing out JNI error code seems over the top (and not very useful).
> 
> I note that this code has not changed, even in the foreign-preview branch, which contains a bunch of JVM cleanups. Maybe we can file a followup (but I'd like to understand better what you are proposing - and what options we have on the table).

I don't have a proposal, just querying the failure strategy. You would need to propagate the fact an error occurred back up to the user code to allow it to decide how to proceed, which would complicate the API.

There are no hooks for printing any stacktrace in the case where they occur in the Thread construction sequence. But -Xlog:exceptions would show them.

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

PR: https://git.openjdk.java.net/panama-foreign/pull/570