[foreign] RFR 8226250: Calling callback from different native threads causes a VM crash

[Maurizio Cimadamore]

Whoops - many thanks for catching!

Maurizio

On 18/06/2019 04:25, Nick Gasson wrote:
> Hi Maurizio,
>
> The patch is missing a declaration for p_env in the AArch64
> SPECIALIZED_HELPER_BODY macro:
>
> diff --git a/src/hotspot/cpu/aarch64/directUpcallHandler_aarch64.cpp b/src/hotspot/cpu/aarch64/directUpcallHandler_aarch64.cpp
> index 913dabd47d..08043b7c86 100644
> --- a/src/hotspot/cpu/aarch64/directUpcallHandler_aarch64.cpp
> +++ b/src/hotspot/cpu/aarch64/directUpcallHandler_aarch64.cpp
> @@ -116,6 +116,7 @@ struct upcall_context {
>     int nlongs = (mask & ARG_MASK);                                       \
>     int ndoubles = ((mask >> ARG_SHIFT) & ARG_MASK);                      \
>     int rettag = RES_TAG;                                                 \
> +  void *p_env = NULL;                                                   \
>                                                                           \
>     JavaCallArguments args((1 + nlongs + ndoubles) * 2);                  \
>                                                                           \
>
> With this change, all the tests pass on AArch64.
>
> Thanks,
> Nick
>
>
>> Hi,
>> Under JNI, it is user responsibility to attach native threads to the VM
>> using attachCurrentThread/detachCurrentThread.
>>
>> Panama doesn't do this and, as a result, if native code creates a new
>> thread and uses it to call a Java callback, an hard crash occurs.
>>
>> This patch adds a call to AttachCurrentThreadAsDaemon before calling the
>> Java callback, only if the native thread has no associated Java thread.
>>
>> The AttachCurrentThreadAsDaemon allows the VM to be shutdown w/o waiting
>> for native threads to complete, which is, I think the semantics we want.
>>
>> We don't call the dual detach method for two reasons:
>>
>> * re-attaching the same thread over and over can be expensive
>>
>> * calling detach after the callback means that we'd lose some register
>> values, so we would additional save/restore
>>
>> Overall, it seems like the 'leak' of JavaThread is not worth the extra
>> complexity?
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8226250_v2/
>>
>> Maurizio