HDFS-16084. Fix getJNIEnv crash due to incorrect state set to tls var

Signed-off-by: Kevin Cai <caixh.kevin@gmail.com>

Author: kevincai

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs-tests/CMakeLists.txt

@@ -42,3 +42,7 @@ add_library(native_mini_dfs
 add_executable(test_native_mini_dfs test_native_mini_dfs.c)
 target_link_libraries(test_native_mini_dfs native_mini_dfs ${JAVA_JVM_LIBRARY})
 add_test(test_test_native_mini_dfs test_native_mini_dfs)
+
+add_executable(test_libhdfs_get_jni test_libhdfs_getjni.c)
+target_link_libraries(test_libhdfs_get_jni hdfs_static)
+add_test(test_test_libhdfs_get_jni test_libhdfs_get_jni)

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs-tests/test_libhdfs_getjni.c

@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <jni.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <hdfs/hdfs.h>
+#include "expect.h"
+
+// hook the jvm runtime function. expect always failure
+_JNI_IMPORT_OR_EXPORT_ jint JNICALL JNI_GetDefaultJavaVMInitArgs(void* args) {
+    return 1;
+}
+
+// hook the jvm runtime function. expect always failure
+_JNI_IMPORT_OR_EXPORT_ jint JNICALL JNI_CreateJavaVM(JavaVM** pvm, void** penv, void* args) {
+    return 1;
+}
+
+int main(int argc, char **argv) {
+    hdfsFS lfs;
+    // connect to nothing, should fail but not crash
+    lfs = hdfsConnectNewInstance(NULL, 0);
+    EXPECT_NULL(lfs);
+    if(lfs) {
+        fprintf(stderr, "Oops! Don't expect the fs to be created successful!");
+        exit(1);
+    } 
+    // try again, should fail but not crash
+    lfs = hdfsConnectNewInstance(NULL, 0);
+    EXPECT_NULL(lfs);
+    if(lfs) {
+        fprintf(stderr, "Oops! Don't expect the fs to be created successful!");
+        exit(1);
+    } 
+    return 0;
+}
+
+/**
+ * vim: ts=4: sw=4: et:
+ */

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/jni_helper.c

@@ -818,26 +818,31 @@ JNIEnv* getJNIEnv(void)
       fprintf(stderr, "getJNIEnv: Unable to create ThreadLocalState\n");
       return NULL;
     }
-    if (threadLocalStorageSet(state)) {
-      mutexUnlock(&jvmMutex);
-      goto fail;
-    }
-    THREAD_LOCAL_STORAGE_SET_QUICK(state);
 
     state->env = getGlobalJNIEnv();
-    mutexUnlock(&jvmMutex);
-
     if (!state->env) {
+        mutexUnlock(&jvmMutex);
         goto fail;
     }
 
     jthrowable jthr = NULL;
     jthr = initCachedClasses(state->env);
     if (jthr) {
+      mutexUnlock(&jvmMutex);
       printExceptionAndFree(state->env, jthr, PRINT_EXC_ALL,
                             "initCachedClasses failed");
       goto fail;
     }
+
+    if (threadLocalStorageSet(state)) {
+      mutexUnlock(&jvmMutex);
+      goto fail;
+    }
+
+    // set the TLS var only when the state passes all the checks
+    THREAD_LOCAL_STORAGE_SET_QUICK(state);
+    mutexUnlock(&jvmMutex);
+
     return state->env;
 
 fail: