RFR (S): 8238676: jni crashes on accessing it from process exit hook

[Anton Tarasov]

Hi David,

Thank you for fixing this! Is it possible to backport the fix to jdk11?

Thanks,
Anton.

On Sat, Feb 29, 2020 at 8:16 AM David Holmes <david.holmes at oracle.com>
wrote:

> Hi Gerard,
>
> Are you okay with this?
>
> Thanks,
> David
>
> On 26/02/2020 10:11 am, David Holmes wrote:
> > Hi Gerard,
> >
> > Thanks for taking a look at this.
> >
> > On 26/02/2020 2:45 am, gerard ziemski wrote:
> >> hi David,
> >>
> >> On 2/18/20 8:04 PM, David Holmes wrote:
> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8238676
> >>> webrev: http://cr.openjdk.java.net/~dholmes/8238676/webrev/
> >>>
> >>> If an application tries to use JNI from an atexit handler, the
> >>> attempt can't succeed (the VM has already logically terminated) but
> >>> it should not crash. The JNI Invocation API code was missing some
> >>> checks in places and wasn't aware of the possibility of trying to
> >>> make calls from the VMThread.
> >>>
> >>> Testing:
> >>>  - new test added for the JNI Invocation API
> >>>  - tiers 1-3
> >>>
> >>> Thanks,
> >>> David
> >>
> >> In jni_GetEnv() we have:
> >>
> >> 4120   if (vm_created == 0) {
> >> 4121     *penv = NULL;
> >> 4122     ret = JNI_EDETACHED;
> >> 4123     return ret;
> >> 4124   }
> >>
> >> but in jni_DetachCurrentThread() we have:
> >>
> >> 4063   if (vm_created == 0) {
> >> 4064     HOTSPOT_JNI_DETACHCURRENTTHREAD_RETURN(JNI_ERR);
> >> 4065     return JNI_ERR;
> >> 4066   }
> >>
> >> should lines 4064,4065 perhaps be:
> >>
> >> 4064     HOTSPOT_JNI_DETACHCURRENTTHREAD_RETURN(JNI_EDETACHED);
> >> 4065     return JNI_EDETACHED;
> >>
> >> to be consistent?
> >
> > Well ... ideally the JNI spec would have considered the question of
> > "what if the JavaVM is no longer live?" and had an JNI_ENOJVM errro
> > code. But it doesn't. Nor does it allow for any JNI method to return
> > JNI_ERR for unexpected errors situations. So we have to adhere to the
> > specifications for each method.
> >
> > For GetEnv the spec only allows for returning JNI_EDETACHED,
> > JNI_EVERSION or JNI_OK. As you can't be attached to a VM that doesn't
> > exist then JNI_EDETACHED is the only possible return code - and it isn't
> > wrong, it just doesn't tell you why you aren't attached.
> >
> > If you apply the same logic to DetachCurrentThread, and follow the spec
> > then it states
> >
> > "Trying to detach a thread that is not attached is a no-op."
> >
> > which suggests we do nothing and report JNI_OK. But I think that would
> > be a disservice to the programmer in the case where the JVM is no longer
> > live. And we're allowed to return "a suitable JNI error code on failure"
> > so that is what I chose to do. And I return JNI_ERR rather than
> > JNI_EDETACHED for two reasons:
> >
> > 1. It is consistent with what we do for attach in the same circumstance:
> >
> > jint JNICALL jni_AttachCurrentThread(JavaVM *vm, void **penv, void
> > *_args) {
> >    HOTSPOT_JNI_ATTACHCURRENTTHREAD_ENTRY(vm, penv, _args);
> >    if (vm_created == 0) {
> >      HOTSPOT_JNI_ATTACHCURRENTTHREAD_RETURN((uint32_t) JNI_ERR);
> >      return JNI_ERR;
> >    }
> >
> > 2. It would be confusing to return JNI_EDETACHED as that means "thread
> > detached from the VM" yet the spec says "Trying to detach a thread that
> > is not attached is a no-op.". So I need to convey that there is a more
> > specific underlying error here.
> >
> >> Looks good otherwise.
> >
> > Thanks for the review.
> >
> > David
> > -----
> >
> >>
> >> cheers
> >>
>