Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1064403 - in /hadoop/common/trunk: CHANGES.txt
 src/java/org/apache/hadoop/util/Shell.java
 src/test/core/org/apache/hadoop/util/TestShell.java
Date: Fri, 28 Jan 2011 01:13:00 -0000
To: common-commits@hadoop.apache.org
From: todd@apache.org
Message-Id: <20110128011300.2EFE923889B3@eris.apache.org>

Author: todd
Date: Fri Jan 28 01:12:59 2011
New Revision: 1064403

URL: http://svn.apache.org/viewvc?rev=1064403&view=rev
Log:
HADOOP-7122. Fix thread leak when shell commands time out. Contributed by Todd Lipcon

Modified:
    hadoop/common/trunk/CHANGES.txt
    hadoop/common/trunk/src/java/org/apache/hadoop/util/Shell.java
    hadoop/common/trunk/src/test/core/org/apache/hadoop/util/TestShell.java

Modified: hadoop/common/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/CHANGES.txt?rev=1064403&r1=1064402&r2=1064403&view=diff
==============================================================================
--- hadoop/common/trunk/CHANGES.txt (original)
+++ hadoop/common/trunk/CHANGES.txt Fri Jan 28 01:12:59 2011
@@ -457,6 +457,8 @@ Release 0.22.0 - Unreleased
 
     HADOOP-7118. Fix NPE in Configuration.writeXml (todd)
 
+    HADOOP-7122. Fix thread leak when shell commands time out. (todd)
+
 Release 0.21.1 - Unreleased
 
   IMPROVEMENTS

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/util/Shell.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/util/Shell.java?rev=1064403&r1=1064402&r2=1064403&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/util/Shell.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/util/Shell.java Fri Jan 28 01:12:59 2011
@@ -205,7 +205,7 @@ abstract public class Shell {
     
     process = builder.start();
     if (timeOutInterval > 0) {
-      timeOutTimer = new Timer();
+      timeOutTimer = new Timer("Shell command timeout");
       timeoutTimerTask = new ShellTimeoutTimerTask(
           this);
       //One time scheduling.
@@ -263,7 +263,7 @@ abstract public class Shell {
     } catch (InterruptedException ie) {
       throw new IOException(ie.toString());
     } finally {
-      if ((timeOutTimer!=null) && !timedOut.get()) {
+      if (timeOutTimer != null) {
         timeOutTimer.cancel();
       }
       // close the input stream

Modified: hadoop/common/trunk/src/test/core/org/apache/hadoop/util/TestShell.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/test/core/org/apache/hadoop/util/TestShell.java?rev=1064403&r1=1064402&r2=1064403&view=diff
==============================================================================
--- hadoop/common/trunk/src/test/core/org/apache/hadoop/util/TestShell.java (original)
+++ hadoop/common/trunk/src/test/core/org/apache/hadoop/util/TestShell.java Fri Jan 28 01:12:59 2011
@@ -24,6 +24,10 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadInfo;
+import java.lang.management.ThreadMXBean;
+import java.util.Timer;
 
 public class TestShell extends TestCase {
 
@@ -95,6 +99,46 @@ public class TestShell extends TestCase 
     shellFile.delete();
     assertTrue("Script didnt not timeout" , shexc.isTimedOut());
   }
+  
+  private static int countTimerThreads() {
+    ThreadMXBean threadBean = ManagementFactory.getThreadMXBean();
+    
+    int count = 0;
+    ThreadInfo[] infos = threadBean.getThreadInfo(threadBean.getAllThreadIds(), 20);
+    for (ThreadInfo info : infos) {
+      if (info == null) continue;
+      for (StackTraceElement elem : info.getStackTrace()) {
+        if (elem.getClassName().contains("Timer")) {
+          count++;
+          break;
+        }
+      }
+    }
+    return count;
+  }
+  
+  public void testShellCommandTimerLeak() throws Exception {
+    String quickCommand[] = new String[] {"/bin/sleep", "100"};
+    
+    int timersBefore = countTimerThreads();
+    System.err.println("before: " + timersBefore);
+    
+    for (int i = 0; i < 10; i++) {
+      Shell.ShellCommandExecutor shexec = new Shell.ShellCommandExecutor(
+            quickCommand, null, null, 1);
+      try {
+        shexec.execute();
+        fail("Bad command should throw exception");
+      } catch (Exception e) {
+        // expected
+      }
+    }
+    Thread.sleep(1000);
+    int timersAfter = countTimerThreads();
+    System.err.println("after: " + timersAfter);
+    assertEquals(timersBefore, timersAfter);
+  }
+  
 
   private void testInterval(long interval) throws IOException {
     Command command = new Command(interval);