Bug 464191 - ssltunnel process persists after Mochitest run. r=jwalden, a=testingonlychange

build/pgo/automation.py.in

@@ -41,10 +41,11 @@ import codecs
 from datetime import datetime
 import itertools
 import logging
-import shutil
 import os
 import re
+import shutil
 import signal
+import subprocess
 import sys
 import threading
 
@@ -104,100 +105,18 @@ log.addHandler(handler)
 # SUBPROCESSING #
 #################
 
-class Process:
+class Process(subprocess.Popen):
   """
-  Represents a subprocess of this process.  We don't just directly use the
-  subprocess module here because we want compatibility with Python 2.3 on
-  non-Windows platforms.  :-(
+  Represents our view of a subprocess.
+  It adds a kill() method which allows it to be stopped explicitly.
   """
 
-  def __init__(self, command, args, env, inputdata = None):
-    """
-    Creates a process representing the execution of the given command, which
-    must be an absolute path, with the given arguments in the given environment.
-    The process is then started.
-    """
-    command = os.path.abspath(command)
-    if IS_WIN32:
-      import tempfile
-      import subprocess
-      
-      if inputdata:
-        inputfile = tempfile.TemporaryFile()
-        inputfile.write(inputdata)
-        inputfile.seek(0)
-      else:
-        inputfile = None
-        
-      cmd = [command]
-      cmd.extend(args)
-      p = subprocess.Popen(cmd, env = env,
-                           stdout = subprocess.PIPE,
-                           stderr = subprocess.STDOUT,
-                           stdin = inputfile)
-      self._out = p.stdout
-    else:
-      import popen2
-      cmd = []
-      if env:
-        for (k, v) in env.iteritems():
-          cmd.append(k + "='" + v + "' ")
-          
-      cmd.append("'" + command + "'")
-      cmd.extend(map(lambda x: "'" + x + "'", args))
-      cmd = " ".join(cmd)
-      p = popen2.Popen4(cmd)
-      self._out = p.fromchild
-
-      if inputdata:
-        p.tochild.write(inputdata)
-        p.tochild.close()
-
-    self._process = p
-    self.pid = p.pid
-    
-    self._thread = threading.Thread(target = lambda: self._run())
-    self._thread.start()
-
-  def _run(self):
-    "Continues execution of this process on a separate thread."
-    p = self._process
-    out = self._out
-
-    if IS_WIN32:
-      running = lambda: p.poll() is None
-    else:
-      running = lambda: p.poll() == -1
-
-    # read in lines until the process finishes, then read in any last remaining
-    # buffered lines
-    while running():
-      line = out.readline().rstrip()
-      if len(line) > 0:
-        log.info(line)
-    for line in out:
-      line = line.rstrip()
-      if len(line) > 0:
-        log.info(line)
-    self._status = p.poll()
-
-  def wait(self):
-    "Waits for this process to finish, then returns the process's status."
-    self._thread.join()
-    return self._status
-
   def kill(self):
-    "Kills this process."
-    try:
-      if not IS_WIN32:
-        os.kill(self._process.pid, signal.SIGKILL)
-      else:
-        import subprocess
-        pid = "%i" % self.pid
-        process = subprocess.Popen(["taskkill", "/F", "/PID", pid])
-        process.wait()
-    except:
-      pass
+    if IS_WIN32:
+      pid = "%i" % self.pid
+      subprocess.Popen(["taskkill", "/F", "/PID", pid]).wait()
+    else:
+      os.kill(self.pid, signal.SIGKILL)
 
 
 #################
@@ -433,7 +352,7 @@ def fillCertificateDB(profileDir):
   certutil = DIST_BIN + "/certutil" + BIN_SUFFIX
   pk12util = DIST_BIN + "/pk12util" + BIN_SUFFIX
 
-  status = Process(certutil, ["-N", "-d", profileDir, "-f", pwfilePath], environment()).wait()
+  status = Process([certutil, "-N", "-d", profileDir, "-f", pwfilePath], env = environment()).wait()
   if status != 0:
     return status
 
@@ -442,13 +361,13 @@ def fillCertificateDB(profileDir):
   for item in files:
     root, ext = os.path.splitext(item)
     if ext == ".ca":
-      Process(certutil, ["-A", "-i", os.path.join(CERTS_SRC_DIR, item),
+      Process([certutil, "-A", "-i", os.path.join(CERTS_SRC_DIR, item),
         "-d", profileDir, "-f", pwfilePath, "-n", root, "-t", "CT,,"],
-        environment()).wait()
+        env = environment()).wait()
     if ext == ".client":
-      Process(pk12util, ["-i", os.path.join(CERTS_SRC_DIR, item), "-w",
+      Process([pk12util, "-i", os.path.join(CERTS_SRC_DIR, item), "-w",
         pwfilePath, "-d", profileDir], 
-        environment()).wait()
+        env = environment()).wait()
 
   os.unlink(pwfilePath)
   return 0
@@ -480,7 +399,7 @@ def runApp(testURL, env, app, profileDir, extraArgs):
   
     # start ssltunnel to provide https:// URLs capability
     ssltunnel = DIST_BIN + "/ssltunnel" + BIN_SUFFIX
-    ssltunnelProcess = Process(ssltunnel, [os.path.join(PROFILE_DIR, "ssltunnel.cfg")], environment())
+    ssltunnelProcess = Process([ssltunnel, os.path.join(PROFILE_DIR, "ssltunnel.cfg")], env = environment())
     log.info("SSL tunnel pid: %d", ssltunnelProcess.pid)
   
   "Run the app, returning the time at which it was started."
@@ -508,8 +427,10 @@ def runApp(testURL, env, app, profileDir, extraArgs):
   else:
     args.append((testURL))
   args.extend(extraArgs)
-  proc = Process(cmd, args, env = environment(env))
+  proc = Process([cmd] + args, env = environment(env), stdout = subprocess.PIPE, stderr = subprocess.STDOUT)
   log.info("Application pid: %d", proc.pid)
+  for line in proc.stdout:
+    log.info(line.rstrip())
   status = proc.wait()
   if status != 0:
     log.info("ERROR FAIL Exited with code %d during test run", status)

build/pgo/genpgocert.py.in

@@ -70,8 +70,11 @@ def dbFilesExist(path):
 
 
 def runUtil(util, args, inputdata = None):
-  proc = automation.Process(util, args, automation.environment(), inputdata)
-  return proc.wait()
+  if inputdata:
+    proc = automation.Process([util] + args, env = automation.environment(), stdin = automation.PIPE)
+    proc.communicate(inputdata)
+    return proc.returncode
+  return automation.Process([util] + args, env = automation.environment()).wait()
 
 
 def createRandomFile(randomFile):

testing/mochitest/runtests.py.in

@@ -222,7 +222,7 @@ class MochitestServer:
             "-f", "./" + "server.js"]
 
     xpcshell = automation.DIST_BIN + "/" + "xpcshell";
-    self._process = automation.Process(xpcshell, args, env = env)
+    self._process = automation.Process([xpcshell] + args, env = env)
     pid = self._process.pid
     if pid < 0:
       print "Error starting server."