[blinkpy] Replace threading.Timer for timing out subprocesses

`Timer` is a non-daemonic thread [0] that `Executive.run_command(...)`
uses to kill a timed out subprocess. When `run_wpt_tests.py` is
interrupted during the manifest update, the timer thread is leaked and
blocks the program from exiting.

This change replaces the timer with the `timeout` argument of
`communicate(...)`, which was added in python3.3. Also, conditionally
reenable tests for the timeout behavior of `Executive`.

[0]: https://github.com/python/cpython/blob/3.11/Lib/threading.py#L1370-L1395

Bug: 1382175
Change-Id: I6c982b62a76a3036f341b6fcc711ab29fe33e78e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4624520
Commit-Queue: Jonathan Lee <jonathanjlee@google.com>
Reviewed-by: Weizhong Xia <weizhong@google.com>
Cr-Commit-Position: refs/heads/main@{#1159429}

third_party/blink/tools/blinkpy/common/system/executive.py

@@ -36,7 +36,6 @@
 import signal
 import subprocess
 import sys
-import threading
 import time
 
 import six
@@ -384,17 +383,16 @@ def run_command(
             env=env,
             close_fds=self._should_close_fds())
 
-        def on_command_timeout():
+        output = b''
+        try:
+            output, _ = process.communicate(string_to_communicate,
+                                            timeout_seconds)
+        except subprocess.TimeoutExpired:
             _log.error('Error: Command timed out after %s seconds',
                        timeout_seconds)
+        finally:
             process.kill()
 
-        if timeout_seconds:
-            timer = threading.Timer(timeout_seconds, on_command_timeout)
-            timer.start()
-
-        output = process.communicate(string_to_communicate)[0]
-
         # run_command automatically decodes to unicode() unless explicitly told not to.
         if decode_output:
             output = output.decode(
@@ -404,9 +402,6 @@ def on_command_timeout():
         # http://bugs.python.org/issue1731717
         exit_code = process.wait()
 
-        if timeout_seconds:
-            timer.cancel()
-
         if debug_logging:
             _log.debug('"%s" took %.2fs', self.command_for_printing(args),
                        time.time() - start_time)

third_party/blink/tools/blinkpy/common/system/executive_unittest.py

@@ -28,6 +28,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import os
+import platform
 import subprocess
 import sys
 import unittest
@@ -188,8 +189,9 @@ def test_kill_process(self):
         # Killing again should fail silently.
         executive.kill_process(process.pid)
 
-    # Flaky on Win. See crbug.com/1242429.
-    def disabled_test_timeout_exceeded(self):
+    @unittest.skipIf(platform.system() == 'Windows',
+                     'Flaky on Win. See crbug.com/1242429.')
+    def test_timeout_exceeded(self):
         executive = Executive()
 
         def timeout():
@@ -199,8 +201,9 @@ def timeout():
         with self.assertRaises(ScriptError):
             timeout()
 
-    # Flaky on Win. See crbug.com/1242429.
-    def disabled_test_timeout_exceeded_exit_code(self):
+    @unittest.skipIf(platform.system() == 'Windows',
+                     'Flaky on Win. See crbug.com/1242429.')
+    def test_timeout_exceeded_exit_code(self):
         executive = Executive()
         exit_code = executive.run_command(
             command_line('sleep', 'infinity'),