You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by nn...@apache.org on 2015/03/09 22:21:14 UTC

mesos git commit: Fixed race in getFieldID helper.

Repository: mesos
Updated Branches:
  refs/heads/master 258b73856 -> e1bae6125


Fixed race in getFieldID helper.

The getFieldID helper looks up the java/lang/NoSuchFieldError class and
stores it in a static. It has turned out to provoke a racy behavior with
Java 8 when multiple drivers are created (and the class object may have
been created by another thread). This patch reverts the 'static'
optimization and looks up the class object when exceptions are thrown.

Review: https://reviews.apache.org/r/31818


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e1bae612
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e1bae612
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e1bae612

Branch: refs/heads/master
Commit: e1bae61258bc80bd6006bbb6d4a4fb5c0cc95820
Parents: 258b738
Author: Niklas Nielsen <ni...@qni.dk>
Authored: Fri Mar 6 17:14:17 2015 -0800
Committer: Niklas Q. Nielsen <ni...@mesosphere.io>
Committed: Mon Mar 9 14:20:55 2015 -0700

----------------------------------------------------------------------
 src/java/jni/convert.cpp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e1bae612/src/java/jni/convert.cpp
----------------------------------------------------------------------
diff --git a/src/java/jni/convert.cpp b/src/java/jni/convert.cpp
index 9f99031..de4a1a2 100644
--- a/src/java/jni/convert.cpp
+++ b/src/java/jni/convert.cpp
@@ -522,20 +522,20 @@ Result<jfieldID> getFieldID(
     const char* name,
     const char* signature)
 {
-  static jclass NO_SUCH_FIELD_ERROR = NULL;
+  jfieldID jfield = env->GetFieldID(clazz, name, signature);
+  jthrowable jexception = env->ExceptionOccurred();
+  if (jexception != NULL) {
+    env->ExceptionClear(); // Clear the exception first before proceeding.
 
-  if (NO_SUCH_FIELD_ERROR == NULL) {
-    NO_SUCH_FIELD_ERROR = env->FindClass("java/lang/NoSuchFieldError");
+    // NOTE: This was previously a static variable in order to only
+    // do the lookup once. But this is not thread-safe and led to
+    // crashes on Java 8!
+    jclass noSuchFieldError = env->FindClass("java/lang/NoSuchFieldError");
     if (env->ExceptionCheck() == JNI_TRUE) {
       return Error("Cannot find NoSuchFieldError class");
     }
-  }
 
-  jfieldID jfield = env->GetFieldID(clazz, name, signature);
-  jthrowable jexception = env->ExceptionOccurred();
-  if (jexception != NULL) {
-    env->ExceptionClear(); // Clear the exception first before proceeding.
-    if (!env->IsInstanceOf(jexception, NO_SUCH_FIELD_ERROR)) {
+    if (!env->IsInstanceOf(jexception, noSuchFieldError)) {
       // We are here if we got a different exception than
       // 'NoSuchFieldError'. Rethrow and bail.
       env->Throw(jexception);