diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/jni_helper.c b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/jni_helper.c index 47dce0086a93c8..04aae7b34c7117 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/jni_helper.c +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/jni_helper.c @@ -651,9 +651,14 @@ static char* getClassPath() * every thread. You must be holding the jvmMutex when you call this * function. * + * @param[out] attachedByLibhdfs Set to true if this call attached the current + * thread to the JVM, false if the thread was + * already attached by someone else. Only the + * former may be detached at thread exit. + * * @return The JNIEnv on success; error code otherwise */ -static JNIEnv* getGlobalJNIEnv(void) +static JNIEnv* getGlobalJNIEnv(bool *attachedByLibhdfs) { JavaVM* vmBuf[VM_BUF_LENGTH]; JNIEnv *env; @@ -672,6 +677,7 @@ static JNIEnv* getGlobalJNIEnv(void) JavaVM *vm; JavaVMOption *options; + *attachedByLibhdfs = false; rv = JNI_GetCreatedJavaVMs(&(vmBuf[0]), VM_BUF_LENGTH, &noVMs); if (rv != 0) { fprintf(stderr, "JNI_GetCreatedJavaVMs failed with error: %d\n", rv); @@ -755,15 +761,30 @@ static JNIEnv* getGlobalJNIEnv(void) "FileSystem: loadFileSystems failed"); return NULL; } + *attachedByLibhdfs = true; } else { - //Attach this thread to the VM vm = vmBuf[0]; + // Reuse an existing attachment rather than creating one. On a thread + // the JVM or the embedding application already attached, + // AttachCurrentThread succeeds and hands back the same JNIEnv, which + // would leave libhdfs believing it owns an attachment it did not make + // and detaching it at thread exit. + rv = (*vm)->GetEnv(vm, (void**)&env, JNI_VERSION_1_2); + if (rv == JNI_OK) { + return env; + } + if (rv != JNI_EDETACHED) { + fprintf(stderr, "Call to GetEnv failed with error: %d\n", rv); + return NULL; + } + //Attach this thread to the VM rv = (*vm)->AttachCurrentThread(vm, (void*)&env, 0); if (rv != 0) { fprintf(stderr, "Call to AttachCurrentThread " "failed with error: %d\n", rv); return NULL; } + *attachedByLibhdfs = true; } return env; @@ -819,7 +840,7 @@ JNIEnv* getJNIEnv(void) return NULL; } - state->env = getGlobalJNIEnv(); + state->env = getGlobalJNIEnv(&state->attachedByLibhdfs); if (!state->env) { mutexUnlock(&jvmMutex); goto fail; diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/os/posix/thread_local_storage.c b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/os/posix/thread_local_storage.c index 1b6dafaba82ea8..ffb710ec25bae1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/os/posix/thread_local_storage.c +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/os/posix/thread_local_storage.c @@ -52,8 +52,11 @@ void hdfsThreadDestructor(void *v) jthrowable jthr; char thr_name[MAXTHRID]; - /* Detach the current thread from the JVM */ - if ((env != NULL) && (*env != NULL)) { + /* Detach only threads that libhdfs attached to the JVM. Detaching a thread + * that the JVM (or an embedding application) attached frees a JNIEnv its + * owner still holds, and by the time this destructor runs that env may + * already have been freed, so the dereference below reads freed memory. */ + if (state->attachedByLibhdfs && (env != NULL) && (*env != NULL)) { ret = (*env)->GetJavaVM(env, &vm); if (ret != 0) { @@ -158,6 +161,8 @@ struct ThreadLocalState* threadLocalStorageCreate() "threadLocalStorageCreate: OOM - Unable to allocate thread local state\n"); return NULL; } + state->attachedByLibhdfs = false; + state->env = NULL; state->lastExceptionStackTrace = NULL; state->lastExceptionRootCause = NULL; return state; diff --git a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/os/thread_local_storage.h b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/os/thread_local_storage.h index 55e4239aa718b2..cc650e95f0ad06 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/os/thread_local_storage.h +++ b/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/os/thread_local_storage.h @@ -28,6 +28,8 @@ #include +#include + /* * Most operating systems support the more efficient __thread construct, which * is initialized by the linker. The following macros use this technique on the @@ -52,6 +54,8 @@ #endif struct ThreadLocalState { + /* Whether libhdfs attached this thread to the JVM. */ + bool attachedByLibhdfs; /* The JNIEnv associated with the current thread */ JNIEnv *env; /* The last exception stack trace that occurred on this thread */