You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by to...@apache.org on 2011/03/11 19:41:54 UTC

svn commit: r1080722 - in /hadoop/common/branches/branch-0.22: ./ src/java/ src/java/org/apache/hadoop/io/nativeio/ src/native/src/org/apache/hadoop/io/nativeio/ src/test/core/org/apache/hadoop/io/nativeio/

Author: todd
Date: Fri Mar 11 18:41:53 2011
New Revision: 1080722

URL: http://svn.apache.org/viewvc?rev=1080722&view=rev
Log:
HADOOP-7156. Workaround for unsafe implementations of getpwuid_r. Contributed by Todd Lipcon.

Modified:
    hadoop/common/branches/branch-0.22/CHANGES.txt
    hadoop/common/branches/branch-0.22/src/java/core-default.xml
    hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/io/nativeio/NativeIO.java
    hadoop/common/branches/branch-0.22/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c
    hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java

Modified: hadoop/common/branches/branch-0.22/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/CHANGES.txt?rev=1080722&r1=1080721&r2=1080722&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/CHANGES.txt (original)
+++ hadoop/common/branches/branch-0.22/CHANGES.txt Fri Mar 11 18:41:53 2011
@@ -427,6 +427,8 @@ Release 0.22.0 - Unreleased
     HADOOP-7145. Configuration.getLocalPath should trim whitespace from
     the provided directories. (todd)
 
+    HADOOP-7156. Workaround for unsafe implementations of getpwuid_r (todd)
+
 Release 0.21.1 - Unreleased
 
   IMPROVEMENTS

Modified: hadoop/common/branches/branch-0.22/src/java/core-default.xml
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/java/core-default.xml?rev=1080722&r1=1080721&r2=1080722&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/java/core-default.xml (original)
+++ hadoop/common/branches/branch-0.22/src/java/core-default.xml Fri Mar 11 18:41:53 2011
@@ -108,6 +108,21 @@
   </description>
 </property>
 
+<property>
+  <name>hadoop.work.around.non.threadsafe.getpwuid</name>
+  <value>false</value>
+  <description>Some operating systems or authentication modules are known to
+  have broken implementations of getpwuid_r and getpwgid_r, such that these
+  calls are not thread-safe. Symptoms of this problem include JVM crashes
+  with a stack trace inside these functions. If your system exhibits this
+  issue, enable this configuration parameter to include a lock around the
+  calls as a workaround.
+
+  An incomplete list of some systems known to have this issue is available
+  at http://wiki.apache.org/hadoop/KnownBrokenPwuidImplementations
+  </description>
+</property>
+
 <!--- logging properties -->
 
 <property>

Modified: hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/io/nativeio/NativeIO.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/io/nativeio/NativeIO.java?rev=1080722&r1=1080721&r2=1080722&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/io/nativeio/NativeIO.java (original)
+++ hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/io/nativeio/NativeIO.java Fri Mar 11 18:41:53 2011
@@ -20,6 +20,7 @@ package org.apache.hadoop.io.nativeio;
 import java.io.FileDescriptor;
 import java.io.IOException;
 
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.util.NativeCodeLoader;
 
 import org.apache.commons.logging.Log;
@@ -48,10 +49,20 @@ public class NativeIO {
   private static final Log LOG = LogFactory.getLog(NativeIO.class);
 
   private static boolean nativeLoaded = false;
+  private static boolean workaroundNonThreadSafePasswdCalls = false;
+
+  static final String WORKAROUND_NON_THREADSAFE_CALLS_KEY =
+    "hadoop.workaround.non.threadsafe.getpwuid";
+  static final boolean WORKAROUND_NON_THREADSAFE_CALLS_DEFAULT = false;
 
   static {
     if (NativeCodeLoader.isNativeCodeLoaded()) {
       try {
+        Configuration conf = new Configuration();
+        workaroundNonThreadSafePasswdCalls = conf.getBoolean(
+          WORKAROUND_NON_THREADSAFE_CALLS_KEY,
+          WORKAROUND_NON_THREADSAFE_CALLS_DEFAULT);
+
         initNative();
         nativeLoaded = true;
       } catch (Throwable t) {

Modified: hadoop/common/branches/branch-0.22/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c?rev=1080722&r1=1080721&r2=1080722&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c (original)
+++ hadoop/common/branches/branch-0.22/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c Fri Mar 11 18:41:53 2011
@@ -44,18 +44,50 @@ static jmethodID stat_ctor;
 static jclass nioe_clazz;
 static jmethodID nioe_ctor;
 
+// the monitor used for working around non-threadsafe implementations
+// of getpwuid_r, observed on platforms including RHEL 6.0.
+// Please see HADOOP-7156 for details.
+static jobject pw_lock_object;
+
 // Internal functions
 static void throw_ioe(JNIEnv* env, int errnum);
 static ssize_t get_pw_buflen();
 
+/**
+ * Returns non-zero if the user has specified that the system
+ * has non-threadsafe implementations of getpwuid_r or getgrgid_r.
+ **/
+static int workaround_non_threadsafe_calls(JNIEnv *env, jclass clazz) {
+  jfieldID needs_workaround_field = (*env)->GetStaticFieldID(env, clazz,
+    "workaroundNonThreadSafePasswdCalls", "Z");
+  PASS_EXCEPTIONS_RET(env, 0);
+  assert(needs_workaround_field);
+
+  jboolean result = (*env)->GetStaticBooleanField(
+    env, clazz, needs_workaround_field);
+  return result;
+}
 
-static void stat_init(JNIEnv *env) {
+static void stat_init(JNIEnv *env, jclass nativeio_class) {
   // Init Stat
   jclass clazz = (*env)->FindClass(env, "org/apache/hadoop/io/nativeio/NativeIO$Stat");
   PASS_EXCEPTIONS(env);
   stat_clazz = (*env)->NewGlobalRef(env, clazz);
   stat_ctor = (*env)->GetMethodID(env, stat_clazz, "<init>",
     "(Ljava/lang/String;Ljava/lang/String;I)V");
+  
+  jclass obj_class = (*env)->FindClass(env, "java/lang/Object");
+  assert(obj_class != NULL);
+  jmethodID  obj_ctor = (*env)->GetMethodID(env, obj_class,
+    "<init>", "()V");
+  assert(obj_ctor != NULL);
+
+  if (workaround_non_threadsafe_calls(env, nativeio_class)) {
+    pw_lock_object = (*env)->NewObject(env, obj_class, obj_ctor);
+    PASS_EXCEPTIONS(env);
+    pw_lock_object = (*env)->NewGlobalRef(env, pw_lock_object);
+    PASS_EXCEPTIONS(env);
+  }
 }
 
 static void stat_deinit(JNIEnv *env) {
@@ -63,6 +95,10 @@ static void stat_deinit(JNIEnv *env) {
     (*env)->DeleteGlobalRef(env, stat_clazz);
     stat_clazz = NULL;
   }
+  if (pw_lock_object != NULL) {
+    (*env)->DeleteGlobalRef(env, pw_lock_object);
+    pw_lock_object = NULL;
+  }
 }
 
 static void nioe_init(JNIEnv *env) {
@@ -95,7 +131,7 @@ JNIEXPORT void JNICALL
 Java_org_apache_hadoop_io_nativeio_NativeIO_initNative(
 	JNIEnv *env, jclass clazz) {
 
-  stat_init(env);
+  stat_init(env, clazz);
   PASS_EXCEPTIONS_GOTO(env, error);
   nioe_init(env);
   PASS_EXCEPTIONS_GOTO(env, error);
@@ -122,6 +158,7 @@ Java_org_apache_hadoop_io_nativeio_Nativ
 {
   jobject ret = NULL;
   char *pw_buf = NULL;
+  int pw_lock_locked = 0;
 
   int fd = fd_get(env, fd_object);
   PASS_EXCEPTIONS_GOTO(env, cleanup);
@@ -139,6 +176,13 @@ Java_org_apache_hadoop_io_nativeio_Nativ
     goto cleanup;
   }
 
+  if (pw_lock_object != NULL) {
+    if ((*env)->MonitorEnter(env, pw_lock_object) != JNI_OK) {
+      goto cleanup;
+    }
+    pw_lock_locked = 1;
+  }
+
   // Grab username
   struct passwd pwd, *pwdp;
   while ((rc = getpwuid_r(s.st_uid, &pwd, pw_buf, pw_buflen, &pwdp)) != 0) {
@@ -183,6 +227,9 @@ Java_org_apache_hadoop_io_nativeio_Nativ
 
 cleanup:
   if (pw_buf != NULL) free(pw_buf);
+  if (pw_lock_locked) {
+    (*env)->MonitorExit(env, pw_lock_object);
+  }
   return ret;
 }
 

Modified: hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java?rev=1080722&r1=1080721&r2=1080722&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java (original)
+++ hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/io/nativeio/TestNativeIO.java Fri Mar 11 18:41:53 2011
@@ -21,6 +21,9 @@ import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.ArrayList;
+import java.util.List;
 import org.junit.Before;
 import org.junit.Test;
 import static org.junit.Assume.*;
@@ -67,6 +70,52 @@ public class TestNativeIO {
       NativeIO.Stat.S_IFREG, stat.getMode() & NativeIO.Stat.S_IFMT);
   }
 
+  /**
+   * Test for races in fstat usage
+   *
+   * NOTE: this test is likely to fail on RHEL 6.0 which has a non-threadsafe
+   * implementation of getpwuid_r.
+   */
+  @Test
+  public void testMultiThreadedFstat() throws Exception {
+    final FileOutputStream fos = new FileOutputStream(
+      new File(TEST_DIR, "testfstat"));
+
+    final AtomicReference<Throwable> thrown =
+      new AtomicReference<Throwable>();
+    List<Thread> statters = new ArrayList<Thread>();
+    for (int i = 0; i < 10; i++) {
+      Thread statter = new Thread() {
+        public void run() {
+          long et = System.currentTimeMillis() + 5000;
+          while (System.currentTimeMillis() < et) {
+            try {
+              NativeIO.Stat stat = NativeIO.fstat(fos.getFD());
+              assertEquals(System.getProperty("user.name"), stat.getOwner());
+              assertNotNull(stat.getGroup());
+              assertTrue(!"".equals(stat.getGroup()));
+              assertEquals("Stat mode field should indicate a regular file",
+                NativeIO.Stat.S_IFREG, stat.getMode() & NativeIO.Stat.S_IFMT);
+            } catch (Throwable t) {
+              thrown.set(t);
+            }
+          }
+        }
+      };
+      statters.add(statter);
+      statter.start();
+    }
+    for (Thread t : statters) {
+      t.join();
+    }
+
+    fos.close();
+    
+    if (thrown.get() != null) {
+      throw new RuntimeException(thrown.get());
+    }
+  }
+
   @Test
   public void testFstatClosedFd() throws Exception {
     FileOutputStream fos = new FileOutputStream(