Kill subprocesses on KeyboardInterrupt.

gclient_utils.py

Index: gclient_utils.py
diff --git a/gclient_utils.py b/gclient_utils.py
index ddcce9b17746568b242c6fead28287c1ea834c5f..b3219350d8a302f9a41cdd6f2cab8e899f5aab4e 100644
--- a/gclient_utils.py
+++ b/gclient_utils.py
@@ -323,6 +323,49 @@ def MakeFileAnnotated(fileobj, include_zero=False):
   return Annotated(fileobj)
 
 
+class GClientChildrenSingleton(object):
+  _instance = None
+  children = {}
+  children_lock = threading.Lock()
+
+  def __new__(cls, *args, **kwargs):
+    if not isinstance(cls._instance, cls):
+      cls._instance = object.__new__(cls, *args, **kwargs)

[M-A Ruel, 2013/05/03 16:39:01]

I don't think it's a good idea. Just create a global variable, it's cleaner and
way more logical.

+    return cls._instance
+
+  def add(self, popen_obj):
+    with self.children_lock:
+      self.children[popen_obj.pid] = popen_obj
+
+  def remove(self, popen_obj):
+    with self.children_lock:
+      del self.children[popen_obj.pid]
+
+
+def KillAllRemainingChildren():
+  children = GClientChildrenSingleton()
+  dead_objs = []
+
+  for pid, child in children.children.iteritems():
+    if child.poll():
+      dead_objs.append(child)
+      continue
+
+    try:
+      child.kill()
+      dead_objs.append(child)
+    except OSError:
+      pass
+
+  for obj in dead_objs:
+    children.remove(obj)
+
+  if children.children:
+    print >> sys.stderr, 'Could not kill the folling subprocesses:'
+    for pid in children.children:
+      print >> sys.stderr, '  ', pid
+
+
 def CheckCallAndFilter(args, stdout=None, filter_fn=None,
                        print_stdout=None, call_filter_on_first_line=False,
                        **kwargs):
@@ -344,6 +387,9 @@ def CheckCallAndFilter(args, stdout=None, filter_fn=None,
       args, bufsize=0, stdout=subprocess2.PIPE, stderr=subprocess2.STDOUT,
       **kwargs)
 
+  children = GClientChildrenSingleton()
+  children.add(kid)
+
   # Do a flush of stdout before we begin reading from the subprocess2's stdout
   stdout.flush()
 
@@ -375,6 +421,11 @@ def CheckCallAndFilter(args, stdout=None, filter_fn=None,
       if len(in_line):
         filter_fn(in_line)
     rv = kid.wait()
+
+    # Don't put this in a 'finally,' since the child may still run if we get an
+    # exception.
+    children.remove(kid)
+
   except KeyboardInterrupt:
     print >> sys.stderr, 'Failed while running "%s"' % ' '.join(args)
     raise
@@ -657,6 +708,7 @@ class ExecutionQueue(object):
       self.index = index
       self.args = args
       self.kwargs = kwargs
+      self.daemon = True
 
     def run(self):
       """Runs in its own thread."""
@@ -664,6 +716,11 @@ class ExecutionQueue(object):
       work_queue = self.kwargs['work_queue']
       try:
         self.item.run(*self.args, **self.kwargs)
+      except KeyboardInterrupt:
+        logging.info('Caught KeyboardInterrupt in thread %s' % self.item.name)
+        logging.info(str(sys.exc_info()))
+        work_queue.exceptions.put(sys.exc_info())
+        raise

[M-A Ruel, 2013/05/03 16:39:01]

Then the lines 731-735 should be in a finally

       except Exception:
         # Catch exception location.
         logging.info('Caught exception in thread %s' % self.item.name)