Bug 814496 - sutAgent: Stop RedirOutputThread when timeout exceeded; r=wlach
authorGeoff Brown <gbrown@mozilla.com>
Tue, 27 Nov 2012 14:05:18 -0700
changeset 114284 7e5deb571bbe010e990cf4f41ce9eafd36310caf
parent 114283 5158d648702ecfe12338765a1c5de00d993189c9
child 114285 415cbab797d5be3728ec9ec6d89a8a4920411b5d
push id23913
push useremorley@mozilla.com
push dateWed, 28 Nov 2012 17:11:31 +0000
treeherdermozilla-central@17c267a881cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswlach
bugs814496
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 814496 - sutAgent: Stop RedirOutputThread when timeout exceeded; r=wlach
build/mobile/sutagent/android/DoCommand.java
build/mobile/sutagent/android/RedirOutputThread.java
--- a/build/mobile/sutagent/android/DoCommand.java
+++ b/build/mobile/sutagent/android/DoCommand.java
@@ -1406,17 +1406,17 @@ private void CancelNotification()
             if (tmpFile.exists()) {
                 sRet = (tmpFile.isDirectory() ? "TRUE" : "FALSE");
             }
             else {
                 try {
                     pProc = Runtime.getRuntime().exec(this.getSuArgs("ls -l " + sDir));
                     RedirOutputThread outThrd = new RedirOutputThread(pProc, null);
                     outThrd.start();
-                    outThrd.join(5000);
+                    outThrd.joinAndStopRedirect(5000);
                     sRet = outThrd.strOutput;
                     if (!sRet.contains("No such file or directory") && sRet.startsWith("l"))
                         sRet = "FALSE";
                 }
                 catch (IOException e) {
                     sRet = e.getMessage();
                     e.printStackTrace();
                 }
@@ -2379,31 +2379,31 @@ private void CancelNotification()
         } else {
             sCmd = "setprop service.adb.tcp.port -1";
         }
 
         try {
             pProc = Runtime.getRuntime().exec(this.getSuArgs(sCmd));
             RedirOutputThread outThrd = new RedirOutputThread(pProc, null);
             outThrd.start();
-            outThrd.join(5000);
+            outThrd.joinAndStopRedirect(5000);
             sTmp = outThrd.strOutput;
             Log.e("ADB", sTmp);
             if (outThrd.nExitCode == 0) {
                 pProc = Runtime.getRuntime().exec(this.getSuArgs("stop adbd"));
                 outThrd = new RedirOutputThread(pProc, null);
                 outThrd.start();
-                outThrd.join(5000);
+                outThrd.joinAndStopRedirect(5000);
                 sTmp = outThrd.strOutput;
                 Log.e("ADB", sTmp);
                 if (outThrd.nExitCode == 0) {
                     pProc = Runtime.getRuntime().exec(this.getSuArgs("start adbd"));
                     outThrd = new RedirOutputThread(pProc, null);
                     outThrd.start();
-                    outThrd.join(5000);
+                    outThrd.joinAndStopRedirect(5000);
                     sTmp = outThrd.strOutput;
                     Log.e("ADB", sTmp);
                     if (outThrd.nExitCode == 0) {
                         sRet = "Successfully set adb to " + sWhat + "\n";
                     } else {
                         sRet = sErrorPrefix + "Failed to start adbd\n";
                     }
                 } else {
@@ -2446,17 +2446,17 @@ private void CancelNotification()
                 sFoundProcName = lProcesses.get(lcv).processName;
                 bFoundAppProcess = true;
 
                 try
                     {
                     pProc = Runtime.getRuntime().exec(this.getSuArgs("kill " + lProcesses.get(lcv).pid));
                     RedirOutputThread outThrd = new RedirOutputThread(pProc, null);
                     outThrd.start();
-                    outThrd.join(15000);
+                    outThrd.joinAndStopRedirect(15000);
                     String sTmp = outThrd.strOutput;
                     Log.e("KILLPROCESS", sTmp);
                     }
                 catch (IOException e)
                     {
                     sRet = e.getMessage();
                     e.printStackTrace();
                     }
@@ -2497,17 +2497,17 @@ private void CancelNotification()
             // To kill processes other than Java applications - processes
             // like xpcshell - a different strategy is necessary: use ps
             // to find the process' PID.
             try
                 {
                 pProc = Runtime.getRuntime().exec("ps");
                 RedirOutputThread outThrd = new RedirOutputThread(pProc, null);
                 outThrd.start();
-                outThrd.join(10000);
+                outThrd.joinAndStopRedirect(10000);
                 String sTmp = outThrd.strOutput;
                 StringTokenizer stokLines = new StringTokenizer(sTmp, "\n");
                 while(stokLines.hasMoreTokens())
                     {
                     String sLine = stokLines.nextToken();
                     StringTokenizer stokColumns = new StringTokenizer(sLine, " \t\n");
                     stokColumns.nextToken();
                     String sPid = stokColumns.nextToken();
@@ -2922,17 +2922,17 @@ private void CancelNotification()
         }
 
         // if we have an argument
         if (sMillis.length() > 0) {
             try {
                 pProc = Runtime.getRuntime().exec(this.getSuArgs("date -u " + sMillis));
                 RedirOutputThread outThrd = new RedirOutputThread(pProc, null);
                 outThrd.start();
-                outThrd.join(10000);
+                outThrd.joinAndStopRedirect(10000);
                 sRet += GetSystemTime();
             } catch (IOException e) {
                 sRet = e.getMessage();
                 e.printStackTrace();
             } catch (InterruptedException e) {
                 e.printStackTrace();
             }
         }
@@ -2987,17 +2987,17 @@ private void CancelNotification()
         {
         String sRet = "";
 
         try
             {
             pProc = Runtime.getRuntime().exec("kill "+sProcId);
             RedirOutputThread outThrd = new RedirOutputThread(pProc, out);
             outThrd.start();
-            outThrd.join(5000);
+            outThrd.joinAndStopRedirect(5000);
             }
         catch (IOException e)
             {
             sRet = e.getMessage();
             e.printStackTrace();
             }
         catch (InterruptedException e)
             {
@@ -3017,17 +3017,17 @@ private void CancelNotification()
         theArgs[2] = "3";
         theArgs[3] = sIPAddr;
 
         try
             {
             pProc = Runtime.getRuntime().exec(theArgs);
             RedirOutputThread outThrd = new RedirOutputThread(pProc, out);
             outThrd.start();
-            outThrd.join(5000);
+            outThrd.joinAndStopRedirect(5000);
             if (out == null)
                 sRet = outThrd.strOutput;
             }
         catch (IOException e)
             {
             sRet = e.getMessage();
             e.printStackTrace();
             }
@@ -3184,17 +3184,17 @@ private void CancelNotification()
 
         try {
             // Tell all of the data channels we are rebooting
             ((ASMozStub)this.contextWrapper).SendToDataChannel("Rebooting ...");
 
             pProc = Runtime.getRuntime().exec(this.getSuArgs("reboot"));
             RedirOutputThread outThrd = new RedirOutputThread(pProc, out);
             outThrd.start();
-            outThrd.join(10000);
+            outThrd.joinAndStopRedirect(10000);
         } catch (IOException e) {
             sRet = e.getMessage();
             e.printStackTrace();
         } catch (InterruptedException e) {
             e.printStackTrace();
         }
 
         return (sRet);
@@ -3221,17 +3221,17 @@ private void CancelNotification()
                 pProc = Runtime.getRuntime().exec(this.getSuArgs("pm uninstall " + sApp + ";reboot;exit"));
             } else {
                 pProc = Runtime.getRuntime().exec(this.getSuArgs("pm uninstall " + sApp + ";exit"));
             }
 
             RedirOutputThread outThrd = new RedirOutputThread(pProc, out);
             outThrd.start();
             try {
-                outThrd.join(60000);
+                outThrd.joinAndStopRedirect(60000);
                 int nRet = pProc.exitValue();
                 sRet = "\nuninst complete [" + nRet + "]";
                 }
             catch (IllegalThreadStateException itse) {
                 itse.printStackTrace();
                 sRet = "\nuninst command timed out";
                 }
             }
@@ -3268,17 +3268,17 @@ private void CancelNotification()
             pProc = Runtime.getRuntime().exec(this.getSuArgs("mv " + GetTmpDir() + "/" +
                                                              srcFile.getName() +
                                                              " /data/local/tmp/" +
                                                              srcFile.getName() + ";exit"));
 
             RedirOutputThread outThrd = new RedirOutputThread(pProc, out);
             outThrd.start();
             try {
-                outThrd.join(90000);
+                outThrd.joinAndStopRedirect(90000);
                 int nRet = pProc.exitValue();
                 sRet = "\nmove complete [" + nRet + "]";
                 }
             catch (IllegalThreadStateException itse) {
                 itse.printStackTrace();
                 sRet = "\nmove command timed out";
             }
             try
@@ -3291,17 +3291,17 @@ private void CancelNotification()
                 e1.printStackTrace();
                 }
 
             pProc = Runtime.getRuntime().exec(this.getSuArgs("chmod 666 /data/local/tmp/" +
                                                              srcFile.getName() + ";exit"));
             RedirOutputThread outThrd2 = new RedirOutputThread(pProc, out);
             outThrd2.start();
             try {
-                outThrd2.join(10000);
+                outThrd2.joinAndStopRedirect(10000);
                 int nRet2 = pProc.exitValue();
                 sRet = "\npermission change complete [" + nRet2 + "]\n";
                 }
             catch (IllegalThreadStateException itse) {
                 itse.printStackTrace();
                 sRet = "\npermission change timed out";
             }
             try {
@@ -3314,17 +3314,17 @@ private void CancelNotification()
                 }
 
             pProc = Runtime.getRuntime().exec(this.getSuArgs("pm install -r /data/local/tmp/" +
                                                              srcFile.getName() + " Cleanup" +
                                                              ";exit"));
             RedirOutputThread outThrd3 = new RedirOutputThread(pProc, out);
             outThrd3.start();
             try {
-                outThrd3.join(60000);
+                outThrd3.joinAndStopRedirect(60000);
                 int nRet3 = pProc.exitValue();
                 sRet = "\ninstallation complete [" + nRet3 + "]";
                 }
             catch (IllegalThreadStateException itse) {
                 itse.printStackTrace();
                 sRet = "\npm install command timed out";
             }
             try {
@@ -3336,17 +3336,17 @@ private void CancelNotification()
                 e1.printStackTrace();
                 }
 
             pProc = Runtime.getRuntime().exec(this.getSuArgs("rm /data/local/tmp/" +
                                                              srcFile.getName() + ";exit"));
             RedirOutputThread outThrd4 = new RedirOutputThread(pProc, out);
             outThrd4.start();
             try {
-                outThrd4.join(60000);
+                outThrd4.joinAndStopRedirect(60000);
                 int nRet4 = pProc.exitValue();
                 sRet = "\ntmp file removed [" + nRet4 + "]";
                 }
             catch (IllegalThreadStateException itse) {
                 itse.printStackTrace();
                 sRet = "\nrm command timed out";
             }
             try {
@@ -3602,16 +3602,17 @@ private void CancelNotification()
                     int nRetCode = pProc.exitValue();
                     sRet = "return code [" + nRetCode + "]";
                     break;
                     }
                 catch (IllegalThreadStateException itse) {
                     lcv++;
                     }
                 }
+            outThrd.stopRedirect();
             }
         catch (IOException e)
             {
             e.printStackTrace();
             }
         catch (InterruptedException e)
             {
             e.printStackTrace();
@@ -3732,16 +3733,17 @@ private void CancelNotification()
                         int nRetCode = pProc.exitValue();
                         sRet = "return code [" + nRetCode + "]";
                         lcv = 30;
                         }
                     catch (IllegalThreadStateException itse) {
                         lcv++;
                         }
                     }
+                outThrd.stopRedirect();
                 }
             else
                 {
                 Intent preIntent = new Intent();
                 for (lcv = 0; lcv < envArray.length; lcv++)
                     {
                     preIntent.putExtra("env" + lcv, envArray[lcv]);
                     }
@@ -3802,17 +3804,17 @@ private void CancelNotification()
                             sRet += "\n" + ChmodDir(sSubDir);
                         }
                         else {
                             // set the new file's permissions to rwxrwxrwx, if possible
                             try {
                                 Process pProc = Runtime.getRuntime().exec("chmod 777 "+files[lcv]);
                                 RedirOutputThread outThrd = new RedirOutputThread(pProc, null);
                                 outThrd.start();
-                                outThrd.join(5000);
+                                outThrd.joinAndStopRedirect(5000);
                                 sRet += "\n\tchmod " + files[lcv].getName() + " ok";
                             } catch (InterruptedException e) {
                                 sRet += "\n\ttimeout waiting for chmod " + files[lcv].getName();
                             } catch (IOException e) {
                                 sRet += "\n\tunable to chmod " + files[lcv].getName();
                             }
                         }
                     }
@@ -3822,17 +3824,17 @@ private void CancelNotification()
                 }
             }
 
         // set the new directory's (or file's) permissions to rwxrwxrwx, if possible
         try {
             Process pProc = Runtime.getRuntime().exec("chmod 777 "+sTmpDir);
             RedirOutputThread outThrd = new RedirOutputThread(pProc, null);
             outThrd.start();
-            outThrd.join(5000);
+            outThrd.joinAndStopRedirect(5000);
             sRet += "\n\tchmod " + sTmpDir + " ok";
         } catch (InterruptedException e) {
             sRet += "\n\ttimeout waiting for chmod " + sTmpDir;
         } catch (IOException e) {
             sRet += "\n\tunable to chmod " + sTmpDir;
         }
 
         return(sRet);
--- a/build/mobile/sutagent/android/RedirOutputThread.java
+++ b/build/mobile/sutagent/android/RedirOutputThread.java
@@ -12,16 +12,18 @@ import java.io.PrintWriter;
 public class RedirOutputThread extends Thread
     {
     OutputStream out;
     InputStream    sutErr;
     InputStream    sutOut;
     Process pProc;
     String    strOutput;
     int    nExitCode = -1;
+    private volatile boolean stopRedirRequested = false;
+    private volatile boolean redirStopped = false;
 
     public RedirOutputThread(Process pProc, OutputStream out)
         {
         super("RedirOutputThread");
         if (pProc != null)
             {
             this.pProc = pProc;
             sutErr = pProc.getErrorStream(); // Stderr
@@ -101,33 +103,53 @@ public class RedirOutputThread extends T
                             pOut.print(sRep);
                             pOut.flush();
                             }
                         else
                             strOutput += sRep;
                         }
                     }
 
-                bStillRunning = (IsProcRunning(pProc) || (sutOut.available() > 0) || (sutErr.available() > 0));
+                bStillRunning = (stopRedirRequested == false) &&
+                    (IsProcRunning(pProc) || (sutOut.available() > 0) || (sutErr.available() > 0));
                 }
             catch (IOException e)
                 {
                 e.printStackTrace();
                 }
             catch (java.lang.IllegalArgumentException e)
                 {
                 // Bug 743766: InputStream.available() unexpectedly throws this sometimes
                 e.printStackTrace();
                 }
             catch (InterruptedException e) 
                 {
                 e.printStackTrace();
                 }
             }
 
+        // notify stopRedirect that redirection has stopped
+        redirStopped = true;
+        if (stopRedirRequested)
+            {
+            synchronized(this) 
+                {
+                notifyAll();
+                }
+            }
+
+        // wait for process to end, if it has not already, then destroy it
+        try
+            {
+            pProc.waitFor();
+            }
+        catch (InterruptedException e) 
+            {
+            e.printStackTrace();
+            }
         pProc.destroy();
         buffer = null;
         System.gc();
         }
 
     private boolean IsProcRunning(Process pProc)
         {
         boolean bRet = false;
@@ -139,9 +161,37 @@ public class RedirOutputThread extends T
         catch (IllegalThreadStateException z)
             {
             nExitCode = -1;
             bRet = true;
             }
 
         return(bRet);
         }
+
+    public synchronized void stopRedirect()
+        {
+        stopRedirRequested = true;
+        // wait for notification that redirection has stopped
+        if (!redirStopped)
+            {
+            try 
+                {
+                // max wait time is somewhat arbitrary and provided "just in case";
+                // we expect to be notified as soon as run() completes its current
+                // sleep and checks the stopRedirRequested flag
+                wait(500);
+                }
+            catch (InterruptedException e) 
+                {
+                e.printStackTrace();
+                }
+            }
+        }
+
+    public void joinAndStopRedirect(long millis) throws InterruptedException
+        {
+        super.join(millis);
+        if (out != null)
+            stopRedirect();
+        }
+
     }