Changeset 55388 in webkit

Timestamp:

Mar 1, 2010 5:07:09 PM (14 years ago)

Author:

dpranke@chromium.org

Message:

2010-03-01 Dirk Pranke <​dpranke@chromium.org>

Reviewed by Eric Seidel.

Work around a bug in Python's subprocess.Popen() that keeps us from
cleaning up DumpRenderTree / test_shell properly when we finish the
tests in new-run-webkit-tests.

​https://bugs.webkit.org/show_bug.cgi?id=35553

• Scripts/webkitpy/layout_tests/port/chromium.py:
• Scripts/webkitpy/layout_tests/port/mac.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/layout_tests/port/chromium.py (modified) (2 diffs)
• Scripts/webkitpy/layout_tests/port/mac.py (modified) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-03-01  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        Work around a bug in Python's subprocess.Popen() that keeps us from
+        cleaning up DumpRenderTree / test_shell properly when we finish the
+        tests in new-run-webkit-tests.
+
+        https://bugs.webkit.org/show_bug.cgi?id=35553
+
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        * Scripts/webkitpy/layout_tests/port/mac.py:
+
 2010-03-01  Arno Renevier  <arno@renevier.net>
 

trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py

@@ -228 +228 @@
         if options:
             cmd += options
+
+        # We need to pass close_fds=True to work around Python bug #2320
+        # (otherwise we can hang when we kill test_shell when we are running
+        # multiple threads). See http://bugs.python.org/issue2320 .
         self._proc = subprocess.Popen(cmd, stdin=subprocess.PIPE,
                                       stdout=subprocess.PIPE,
-                                      stderr=subprocess.STDOUT)
-
+                                      stderr=subprocess.STDOUT,
+                                      close_fds=True)
     def poll(self):
         return self._proc.poll()
@@ -300 +304 @@
             if self._proc.stderr:
                 self._proc.stderr.close()
-            if (sys.platform not in ('win32', 'cygwin') and
-                not self._proc.poll()):
-                # Closing stdin/stdout/stderr hangs sometimes on OS X.
-                null = open(os.devnull, "w")
-                subprocess.Popen(["kill", "-9",
-                                 str(self._proc.pid)], stderr=null)
-                null.close()
+            if sys.platform not in ('win32', 'cygwin'):
+                # Closing stdin/stdout/stderr hangs sometimes on OS X,
+                # (see __init__(), above), and anyway we don't want to hang
+                # the harness if test_shell is buggy, so we wait a couple
+                # seconds to give test_shell a chance to clean up, but then
+                # force-kill the process if necessary.
+                KILL_TIMEOUT = 3.0
+                timeout = time.time() + KILL_TIMEOUT
+                while self._proc.poll() is None and time.time() < timeout:
+                    time.sleep(0.1)
+                if self._proc.poll() is None:
+                    logging.warning('stopping test driver timed out, '
+                                    'killing it')
+                    null = open(os.devnull, "w")
+                    subprocess.Popen(["kill", "-9",
+                                     str(self._proc.pid)], stderr=null)
+                    null.close()

trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/mac.py

@@ -322 +322 @@
     def restart(self):
         self.stop()
+        # We need to pass close_fds=True to work around Python bug #2320
+        # (otherwise we can hang when we kill test_shell when we are running
+        # multiple threads). See http://bugs.python.org/issue2320 .
         self._proc = subprocess.Popen(self._cmd, stdin=subprocess.PIPE,
                                       stdout=subprocess.PIPE,
                                       stderr=subprocess.PIPE,
-                                      env=self._env)
+                                      env=self._env,
+                                      close_fds=True)
 
     def returncode(self):
@@ -426 +430 @@
             if self._proc.stderr:
                 self._proc.stderr.close()
-            if (sys.platform not in ('win32', 'cygwin') and
-                not self._proc.poll()):
-                # Closing stdin/stdout/stderr hangs sometimes on OS X.
-                null = open(os.devnull, "w")
-                subprocess.Popen(["kill", "-9",
-                                 str(self._proc.pid)], stderr=null)
-                null.close()
+            if sys.platform not in ('win32', 'cygwin'):
+                # Closing stdin/stdout/stderr hangs sometimes on OS X,
+                # (see restart(), above), and anyway we don't want to hang
+                # the harness if test_shell is buggy, so we wait a couple
+                # seconds to give test_shell a chance to clean up, but then
+                # force-kill the process if necessary.
+                KILL_TIMEOUT = 3.0
+                timeout = time.time() + KILL_TIMEOUT
+                while self._proc.poll() is None and time.time() < timeout:
+                    time.sleep(0.1)
+                if self._proc.poll() is None:
+                    logging.warning('stopping test driver timed out, '
+                                    'killing it')
+                    null = open(os.devnull, "w")
+                    subprocess.Popen(["kill", "-9",
+                                     str(self._proc.pid)], stderr=null)
+                    null.close()                not self._proc.poll()):
 
     def _read_line(self, timeout, stop_time, image_length=0):