diff --git a/CHANGES.txt b/CHANGES.txt index 5c3f12994d7..f41d6f862fb 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -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 diff --git a/src/java/org/apache/hadoop/util/Shell.java b/src/java/org/apache/hadoop/util/Shell.java index 3ba119d66e1..1b35ccfc391 100644 --- a/src/java/org/apache/hadoop/util/Shell.java +++ b/src/java/org/apache/hadoop/util/Shell.java @@ -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 diff --git a/src/test/core/org/apache/hadoop/util/TestShell.java b/src/test/core/org/apache/hadoop/util/TestShell.java index 1550db098a7..07ca9179c25 100644 --- a/src/test/core/org/apache/hadoop/util/TestShell.java +++ b/src/test/core/org/apache/hadoop/util/TestShell.java @@ -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);