From 00ee6fdea3efc52b253259cb79e8fe1bc94d4448 Mon Sep 17 00:00:00 2001
From: Geoff Brown <gbrown@mozilla.com>
Date: Mon, 13 Nov 2017 13:12:05 -0700
Subject: [PATCH] Bug 1415290 - Check return codes in mozcrash kill_pid;
 r=jmaher

---
 testing/mochitest/runtests.py                 | 35 ++++++++++++++++---
 testing/mozbase/mozcrash/mozcrash/mozcrash.py | 24 ++++++++++++-
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/testing/mochitest/runtests.py b/testing/mochitest/runtests.py
index 4e455d9a15d0..b1b7eeea56b9 100644
--- a/testing/mochitest/runtests.py
+++ b/testing/mochitest/runtests.py
@@ -1981,11 +1981,15 @@ toolbar#nav-bar {
         If parent_pid is provided, and psutil is available, returns children of
         parent_pid according to psutil.
         """
+        rv = []
         if parent_pid and HAVE_PSUTIL:
             self.log.info("Determining child pids from psutil")
-            return [p.pid for p in psutil.Process(parent_pid).children()]
+            try:
+                rv = [p.pid for p in psutil.Process(parent_pid).children()]
+            except psutil.NoSuchProcess:
+                self.log.warning("Failed to lookup children of pid %d" % parent_pid)
+            return rv
 
-        rv = []
         pid_re = re.compile(r'==> process \d+ launched child process (\d+)')
         with open(process_log) as fd:
             for line in fd:
@@ -2231,6 +2235,10 @@ toolbar#nav-bar {
             # until bug 913970 is fixed regarding mozrunner `wait` not returning status
             # see https://bugzilla.mozilla.org/show_bug.cgi?id=913970
             status = proc.wait()
+            if status is None:
+                self.log.warning("runtests.py | Failed to get app exit code - running/crashed?")
+                # must report an integer to process_exit()
+                status = 0
             self.log.process_exit("Main app process", status)
             runner.process_handler = None
 
@@ -2788,7 +2796,16 @@ toolbar#nav-bar {
         self.log.info('Found child pids: %s' % child_pids)
 
         if HAVE_PSUTIL:
-            child_procs = [psutil.Process(pid) for pid in child_pids]
+            try:
+                browser_proc = [psutil.Process(browser_pid)]
+            except:
+                self.log.info('Failed to get proc for pid %d' % browser_pid)
+                browser_proc = []
+            try:
+                child_procs = [psutil.Process(pid) for pid in child_pids]
+            except:
+                self.log.info('Failed to get child procs')
+                child_procs = []
             for pid in child_pids:
                 self.killAndGetStack(pid, utilityPath, debuggerInfo,
                                      dump_screen=not debuggerInfo)
@@ -2798,6 +2815,14 @@ toolbar#nav-bar {
             for p in alive:
                 self.log.warning('failed to kill pid %d after 30s' %
                                  p.pid)
+            self.killAndGetStack(browser_pid, utilityPath, debuggerInfo,
+                                 dump_screen=not debuggerInfo)
+            gone, alive = psutil.wait_procs(browser_proc, timeout=30)
+            for p in gone:
+                self.log.info('psutil found pid %s dead' % p.pid)
+            for p in alive:
+                self.log.warning('failed to kill pid %d after 30s' %
+                                 p.pid)
         else:
             self.log.error("psutil not available! Will wait 30s before "
                            "attempting to kill parent process. This should "
@@ -2808,8 +2833,8 @@ toolbar#nav-bar {
             if child_pids:
                 time.sleep(30)
 
-        self.killAndGetStack(browser_pid, utilityPath, debuggerInfo,
-                             dump_screen=not debuggerInfo)
+            self.killAndGetStack(browser_pid, utilityPath, debuggerInfo,
+                                 dump_screen=not debuggerInfo)
 
     class OutputHandler(object):
 
diff --git a/testing/mozbase/mozcrash/mozcrash/mozcrash.py b/testing/mozbase/mozcrash/mozcrash/mozcrash.py
index 4642741d6f24..3f3ad7326c4d 100644
--- a/testing/mozbase/mozcrash/mozcrash/mozcrash.py
+++ b/testing/mozbase/mozcrash/mozcrash/mozcrash.py
@@ -473,10 +473,32 @@ if mozinfo.isWin:
         :param pid: PID of the process to terminate.
         """
         PROCESS_TERMINATE = 0x0001
+        WAIT_OBJECT_0 = 0x0
+        WAIT_FAILED = -1
+        logger = get_logger()
         handle = OpenProcess(PROCESS_TERMINATE, 0, pid)
         if handle:
-            kernel32.TerminateProcess(handle, 1)
+            if kernel32.TerminateProcess(handle, 1):
+                # TerminateProcess is async; wait up to 30 seconds for process to
+                # actually terminate, then give up so that clients are not kept
+                # waiting indefinitely for hung processes.
+                status = kernel32.WaitForSingleObject(handle, 30000)
+                if status == WAIT_FAILED:
+                    err = kernel32.GetLastError()
+                    logger.warning("kill_pid(): wait failed (%d) terminating pid %d: error %d" %
+                                   (status, pid, err))
+                elif status != WAIT_OBJECT_0:
+                    logger.warning("kill_pid(): wait failed (%d) terminating pid %d" %
+                                   (status, pid))
+            else:
+                err = kernel32.GetLastError()
+                logger.warning("kill_pid(): unable to terminate pid %d: %d" %
+                               (pid, err))
             CloseHandle(handle)
+        else:
+            err = kernel32.GetLastError()
+            logger.warning("kill_pid(): unable to get handle for pid %d: %d" %
+                           (pid, err))
 else:
     def kill_pid(pid):
         """