Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8280422

thread_from_jni_environment can never return NULL

    XMLWordPrintable

Details

    • Enhancement
    • Resolution: Fixed
    • P4
    • 19
    • 19
    • hotspot
    • None
    • b07

    Description

      Summary: thread_from_jni_environment can never return NULL, so we can delete that and keep static analysis tools happy.

      We have the following code:

      static JavaThread* thread_from_jni_environment(JNIEnv* env) {
          JavaThread *thread_from_jni_env = (JavaThread*)((intptr_t)env - in_bytes(jni_environment_offset()));
          // Only return NULL if thread is off the thread list; starting to
          // exit should not return NULL.
          if (thread_from_jni_env->is_terminated()) {
            thread_from_jni_env->block_if_vm_exited();
            return NULL;
          } else {
            return thread_from_jni_env;
          }
      }

      The "return NULL" causes consternation for static analysis tools that don't understand the broader context.

      This form of the code dates back to early 2002 when it was added as part of JDK-4526887 which introduced the behaviour of the VM terminating at a safepoint.

      Note that the calculated JavaThread* can never be NULL with a valid JNIEnv* as the JNIEnv is an embedded struct within a JavaThread instance. Hence the JNIEnv is only valid for use in that thread and that is always the current thread. The JNIEnv* is extracted inside the VM from the current JavaThread when needed for making native calls (JNI Java->Native, JVM entry points, JRT entry points and Unsafe entry points) and is always valid in those cases.

      The case where a JNIEnv* may not be valid is when used by external native code to make a JNI function call. The JNIEnv* is handed out as part of the JNI_AttachCurrentThread operation and thereafter is used to make JNI function calls e.g.

      env->FindClass(env, "Foo");

      (depending on whether C or C++ used the passing of "env" as the first parameter may be implicit or explicit). If the JNIEnv* does not belong to the current thread, or the current thread has detached since acquiring the JNIEnv* then it is a programming error and JNI does not attempt to detect such programming errors in general.

      Note that the function does not encounter a NULL thread reference but chooses to return NULL if the thread has terminated (and the VM has not exited else we won't reach the return). Encountering a truly terminated thread in this code should be impossible as it would require making a JNI/JRT/JVM/Unsafe call from the thread _after_ it has reached the late stages of termination and we simply do not, and cannot, do that - the thread is already invisible to safepoints by that stage and is very constrained in what it is allowed to do. In the case of a JNI attached thread, it is only marked terminated once it has detached, so again we should never reach this code with a terminated thread and a valid JNIEnv (note that after detaching, the JNIEnv* points to deallocated memory). However a thread is also considered to be terminated if the VM has exited, in which case we must call block_if_vm_exited to prevent daemon threads from re-entering a terminated VM. Consequently we can simply return the JavaThread* which can never be NULL - thus soothing the concerns of static analysis tools.

      We also adjust a sanity check that is applied at the external JNI entry points. That sanity check is currently:

          JavaThread* thread=JavaThread::thread_from_jni_environment(env); \
      - assert( !VerifyJNIEnvThread || (thread == Thread::current()), "JNIEnv is only valid in same thread"); \

      where VerifyJNIEnvThread is a notproduct flag that is false by default - hence unless we explicitly request it, we don't check that the extracted thread is in fact the same as Thread::current(). The adjustment is to get rid of VerifyJNIEnvThread and always check we really do have the right JNIEnv*. This change really only impacts our own testing.

      Attachments

        Issue Links

          Activity

            People

              dholmes David Holmes
              dholmes David Holmes
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: