[llvm] 6f8c450 - [lit] Cleanly exit on user keyboard interrupt

[Julian Lettner via llvm-commits]

Author: Julian Lettner
Date: 2020-04-03T13:03:44-07:00
New Revision: 6f8c45067b1d8d18a669302c0a14d37a230fbc42

URL: https://github.com/llvm/llvm-project/commit/6f8c45067b1d8d18a669302c0a14d37a230fbc42
DIFF: https://github.com/llvm/llvm-project/commit/6f8c45067b1d8d18a669302c0a14d37a230fbc42.diff

LOG: [lit] Cleanly exit on user keyboard interrupt

Graceful lit shutdown on user keyboard interrupt [Ctrl+C] was a
longstanding goal of mine.  After a few refactorings this revision
finally enables it.  We use the following strategy to deal with
KeyboardInterrupt:
https://noswap.com/blog/python-multiprocessing-keyboardinterrupt

Printing of a helpful summary for interrupted runs (just as the one for
completed runs) will be tackled in future revisions.

Reviewed By: serge-sans-paille, rnk

Differential Revision: https://reviews.llvm.org/D77365

Added: 
    

Modified: 
    llvm/utils/lit/lit/main.py
    llvm/utils/lit/lit/run.py
    llvm/utils/lit/lit/util.py
    llvm/utils/lit/lit/worker.py

Removed: 
    


################################################################################
diff  --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py
index 8ac53dfbad9b..fb45995a6280 100755
--- a/llvm/utils/lit/lit/main.py
+++ b/llvm/utils/lit/lit/main.py
@@ -232,10 +232,6 @@ def execute_in_tmp_dir(run, lit_config):
                 'TEMP': tmp_dir,
                 'TEMPDIR': tmp_dir,
                 })
-    # FIXME: If Python does not exit cleanly, this directory will not be cleaned
-    # up. We should consider writing the lit pid into the temp directory,
-    # scanning for stale temp directories, and deleting temp directories whose
-    # lit process has died.
     try:
         run.execute()
     finally:

diff  --git a/llvm/utils/lit/lit/run.py b/llvm/utils/lit/lit/run.py
index 2fa8ebfd9feb..fca3fb47fd5e 100644
--- a/llvm/utils/lit/lit/run.py
+++ b/llvm/utils/lit/lit/run.py
@@ -73,16 +73,9 @@ def _execute(self, deadline):
 
         self._increase_process_limit()
 
-        # Start a process pool. Copy over the data shared between all test runs.
-        # FIXME: Find a way to capture the worker process stderr. If the user
-        # interrupts the workers before we make it into our task callback, they
-        # will each raise a KeyboardInterrupt exception and print to stderr at
-        # the same time.
         pool = multiprocessing.Pool(self.workers, lit.worker.initialize,
                                     (self.lit_config, semaphores))
 
-        self._install_win32_signal_handler(pool)
-
         async_results = [
             pool.apply_async(lit.worker.execute, args=[test],
                              callback=self.progress_callback)
@@ -135,13 +128,3 @@ def _increase_process_limit(self):
             # Warn, unless this is Windows, in which case this is expected.
             if os.name != 'nt':
                 self.lit_config.warning('Failed to raise process limit: %s' % ex)
-
-    def _install_win32_signal_handler(self, pool):
-        if lit.util.win32api is not None:
-            def console_ctrl_handler(type):
-                print('\nCtrl-C detected, terminating.')
-                pool.terminate()
-                pool.join()
-                lit.util.abort_now()
-                return True
-            lit.util.win32api.SetConsoleCtrlHandler(console_ctrl_handler, True)

diff  --git a/llvm/utils/lit/lit/util.py b/llvm/utils/lit/lit/util.py
index fa752d0e8d6e..ab51988e21ec 100644
--- a/llvm/utils/lit/lit/util.py
+++ b/llvm/utils/lit/lit/util.py
@@ -479,17 +479,3 @@ def killProcessAndChildren(pid):
             psutilProc.kill()
         except psutil.NoSuchProcess:
             pass
-
-
-try:
-    import win32api
-except ImportError:
-    win32api = None
-
-def abort_now():
-    """Abort the current process without doing any exception teardown"""
-    sys.stdout.flush()
-    if win32api:
-        win32api.TerminateProcess(win32api.GetCurrentProcess(), 3)
-    else:
-        os.kill(0, 9)

diff  --git a/llvm/utils/lit/lit/worker.py b/llvm/utils/lit/lit/worker.py
index d4364c3dcca4..95a8578afac9 100644
--- a/llvm/utils/lit/lit/worker.py
+++ b/llvm/utils/lit/lit/worker.py
@@ -5,6 +5,7 @@
 For efficiency, we copy all data needed to execute all tests into each worker
 and store it in global variables. This reduces the cost of each task.
 """
+import signal
 import time
 import traceback
 
@@ -23,6 +24,11 @@ def initialize(lit_config, parallelism_semaphores):
     _lit_config = lit_config
     _parallelism_semaphores = parallelism_semaphores
 
+    # We use the following strategy for dealing with Ctrl+C/KeyboardInterrupt in
+    # subprocesses created by the multiprocessing.Pool.
+    # https://noswap.com/blog/python-multiprocessing-keyboardinterrupt
+    signal.signal(signal.SIGINT, signal.SIG_IGN)
+
 
 def execute(test):
     """Run one test in a multiprocessing.Pool
@@ -33,16 +39,10 @@ def execute(test):
     Arguments and results of this function are pickled, so they should be cheap
     to copy.
     """
-    try:
-        result = _execute_in_parallelism_group(test, _lit_config,
-                                               _parallelism_semaphores)
-        test.setResult(result)
-        return test
-    except KeyboardInterrupt:
-        # If a worker process gets an interrupt, abort it immediately.
-        lit.util.abort_now()
-    except:
-        traceback.print_exc()
+    result = _execute_in_parallelism_group(test, _lit_config,
+                                           _parallelism_semaphores)
+    test.setResult(result)
+    return test
 
 
 def _execute_in_parallelism_group(test, lit_config, parallelism_semaphores):
@@ -72,8 +72,6 @@ def _execute_test_handle_errors(test, lit_config):
     try:
         result = test.config.test_format.execute(test, lit_config)
         return _adapt_result(result)
-    except KeyboardInterrupt:
-        raise
     except:
         if lit_config.debug:
             raise