[testing] Fix signal handling on Windows.

See https://bugs.chromium.org/p/chromium/issues/detail?id=733612#c6
for details.

Also add tests both for windows and non-windows.

Bug: 733612
Change-Id: Id656635a5fc79ea8f805658873599b526e764406
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796572Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: John Chen <johnchen@chromium.org>
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695429}

testing/test_env.py

@@ -189,8 +189,8 @@ def run_command_with_output(argv, stdoutfile, env=None, cwd=None):
   assert stdoutfile
   with io.open(stdoutfile, 'wb') as writer, \
       io.open(stdoutfile, 'rb', 1) as reader:
-    process = subprocess.Popen(argv, env=env, cwd=cwd, stdout=writer,
-        stderr=subprocess.STDOUT)
+    process = _popen(argv, env=env, cwd=cwd, stdout=writer,
+                     stderr=subprocess.STDOUT)
     forward_signals([process])
     while process.poll() is None:
       sys.stdout.write(reader.read())
@@ -214,7 +214,7 @@ def run_command(argv, env=None, cwd=None, log=True):
   """
   if log:
     print('Running %r in %r (env: %r)' % (argv, cwd, env))
-  process = subprocess.Popen(argv, env=env, cwd=cwd, stderr=subprocess.STDOUT)
+  process = _popen(argv, env=env, cwd=cwd, stderr=subprocess.STDOUT)
   forward_signals([process])
   return wait_with_signals(process)
 
@@ -229,7 +229,7 @@ def run_command_output_to_handle(argv, file_handle, env=None, cwd=None):
     integer returncode of the subprocess.
   """
   print('Running %r in %r (env: %r)' % (argv, cwd, env))
-  process = subprocess.Popen(
+  process = _popen(
       argv, env=env, cwd=cwd, stderr=file_handle, stdout=file_handle)
   forward_signals([process])
   exit_code = wait_with_signals(process)
@@ -356,9 +356,9 @@ def run_executable(cmd, env, stdoutfile=None):
     elif use_symbolization_script:
       # See above comment regarding offline symbolization.
       # Need to pipe to the symbolizer script.
-      p1 = subprocess.Popen(cmd, env=env, stdout=subprocess.PIPE,
-                            stderr=sys.stdout)
-      p2 = subprocess.Popen(
+      p1 = _popen(cmd, env=env, stdout=subprocess.PIPE,
+                  stderr=sys.stdout)
+      p2 = _popen(
           get_sanitizer_symbolize_command(executable_path=cmd[0]),
           env=env, stdin=p1.stdout)
       p1.stdout.close()  # Allow p1 to receive a SIGPIPE if p2 exits.
@@ -375,6 +375,14 @@ def run_executable(cmd, env, stdoutfile=None):
     raise
 
 
+def _popen(*args, **kwargs):
+  assert 'creationflags' not in kwargs
+  if sys.platform == 'win32':
+    # Necessary for signal handling. See crbug.com/733612#c6.
+    kwargs['creationflags'] = subprocess.CREATE_NEW_PROCESS_GROUP
+  return subprocess.Popen(*args, **kwargs)
+
+
 def main():
   return run_executable(sys.argv[1:], os.environ.copy())
 

testing/test_env_test_script.py

+#!/usr/bin/env python
+# Copyright (c) 2019 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Script for use in test_env unittests."""
+
+import os
+import signal
+import sys
+import time
+
+
+def print_signal(sig, *_):
+  print 'Signal :{}'.format(sig)
+
+
+if __name__ == '__main__':
+  signal.signal(signal.SIGTERM, print_signal)
+  signal.signal(signal.SIGINT, print_signal)
+  if sys.platform == 'win32':
+    signal.signal(signal.SIGBREAK, print_signal)
+  time.sleep(2)  # gives process time to receive signal.

testing/test_env_unittest.py

+#!/usr/bin/env python
+# Copyright (c) 2019 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Unit tests for test_env.py functionality.
+
+Each unit test is launches python process that uses test_env.py
+to launch another python process. Then signal handling and
+propagation is tested. This similates how Swarming uses test_env.py.
+"""
+
+import os
+import signal
+import subprocess
+import sys
+import time
+import unittest
+
+
+TEST_SCRIPT = 'test_env_user_script.py'
+
+
+def launch_process_windows(args):
+  return subprocess.Popen(
+      [sys.executable, TEST_SCRIPT] + args, stdout=subprocess.PIPE,
+      stderr=subprocess.STDOUT, env=os.environ.copy(),
+      creationflags=subprocess.CREATE_NEW_PROCESS_GROUP)
+
+def launch_process_nonwindows(args):
+  return subprocess.Popen(
+      [sys.executable, TEST_SCRIPT] + args, stdout=subprocess.PIPE,
+      stderr=subprocess.STDOUT, env=os.environ.copy())
+
+
+def read_subprocess_message(proc, starts_with):
+  """Finds the value after first line prefix condition."""
+  for line in proc.stdout:
+    if line.startswith(starts_with):
+      return line.rstrip().replace(starts_with, '')
+
+
+def send_and_wait(proc, sig, sleep_time=0.3):
+  """Sends a signal to subprocess."""
+  time.sleep(sleep_time)  # gives process time to launch.
+  os.kill(proc.pid, sig)
+  proc.wait()
+
+
+class SignalingWindowsTest(unittest.TestCase):
+
+  def setUp(self):
+    super(SignalingWindowsTest, self).setUp()
+    if sys.platform != 'win32':
+      self.skipTest('test only runs on Windows')
+
+  def test_send_ctrl_break_event(self):
+    proc = launch_process_windows([])
+    send_and_wait(proc, signal.CTRL_BREAK_EVENT)
+    sig = read_subprocess_message(proc, 'Signal :')
+    self.assertEqual(sig, str(signal.SIGBREAK))
+
+
+class SignalingNonWindowsTest(unittest.TestCase):
+
+  def setUp(self):
+    super(SignalingNonWindowsTest, self).setUp()
+    if sys.platform == 'win32':
+      self.skipTest('test does not run on Windows')
+
+  def test_send_sigterm(self):
+    proc = launch_process_nonwindows([])
+    send_and_wait(proc, signal.SIGTERM)
+    sig = read_subprocess_message(proc, 'Signal :')
+    self.assertEqual(sig, str(signal.SIGTERM))
+
+  def test_send_sigint(self):
+    proc = launch_process_nonwindows([])
+    send_and_wait(proc, signal.SIGINT)
+    sig = read_subprocess_message(proc, 'Signal :')
+    self.assertEqual(sig, str(signal.SIGINT))
+
+
+if __name__ == '__main__':
+  unittest.main()

testing/test_env_user_script.py

+#!/usr/bin/env python
+# Copyright (c) 2019 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Script for use in test_env unittests."""
+
+import test_env
+
+
+if __name__ == '__main__':
+  test_env.run_command(['python', 'test_env_test_script.py'])