[Lldb-commits] [lldb] r248936 - Fixes a potential hang in test runner timeout logic.

Todd Fiala via lldb-commits lldb-commits at lists.llvm.org
Wed Sep 30 13:13:58 PDT 2015


Author: tfiala
Date: Wed Sep 30 15:13:58 2015
New Revision: 248936

URL: http://llvm.org/viewvc/llvm-project?rev=248936&view=rev
Log:
Fixes a potential hang in test runner timeout logic.

Part of https://llvm.org/bugs/show_bug.cgi?id=25002

In writing the new process_control test included here,
I discovered that the scenario would hang indefinitely,
bypassing the timeout logic.

This fixes the indefinite hang, converting an error
in the new test to a failure.  I'll fix the failure
next.  This one was heinous enough to fix on its own,
though.

Modified:
    lldb/trunk/test/test_runner/lib/process_control.py
    lldb/trunk/test/test_runner/test/inferior.py
    lldb/trunk/test/test_runner/test/process_control_tests.py

Modified: lldb/trunk/test/test_runner/lib/process_control.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/test_runner/lib/process_control.py?rev=248936&r1=248935&r2=248936&view=diff
==============================================================================
--- lldb/trunk/test/test_runner/lib/process_control.py (original)
+++ lldb/trunk/test/test_runner/lib/process_control.py Wed Sep 30 15:13:58 2015
@@ -297,17 +297,23 @@ class UnixProcessHelper(ProcessHelper):
                 log_file.write("skipping soft_terminate(): no process id")
             return False
 
-        # Don't kill if it's already dead.
-        popen_process.poll()
-        if popen_process.returncode is not None:
-            # It has a returncode.  It has already stopped.
-            if log_file:
-                log_file.write(
-                    "requested to terminate pid {} but it has already "
-                    "terminated, returncode {}".format(
-                        popen_process.pid, popen_process.returncode))
-            # Move along...
-            return False
+        # We only do the process liveness check if we're not using
+        # process groups.  With process groups, checking if the main
+        # inferior process is dead and short circuiting here is no
+        # good - children of it in the process group could still be
+        # alive, and they should be killed during a timeout.
+        if not popen_process.using_process_groups:
+            # Don't kill if it's already dead.
+            popen_process.poll()
+            if popen_process.returncode is not None:
+                # It has a returncode.  It has already stopped.
+                if log_file:
+                    log_file.write(
+                        "requested to terminate pid {} but it has already "
+                        "terminated, returncode {}".format(
+                            popen_process.pid, popen_process.returncode))
+                # Move along...
+                return False
 
         # Good to go.
         return True

Modified: lldb/trunk/test/test_runner/test/inferior.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/test_runner/test/inferior.py?rev=248936&r1=248935&r2=248936&view=diff
==============================================================================
--- lldb/trunk/test/test_runner/test/inferior.py (original)
+++ lldb/trunk/test/test_runner/test/inferior.py Wed Sep 30 15:13:58 2015
@@ -3,6 +3,7 @@
 import argparse
 import datetime
 import signal
+import subprocess
 import sys
 import time
 
@@ -25,6 +26,15 @@ def parse_args(command_line):
         default=[],
         help="ignore the given signal number (if possible)")
     parser.add_argument(
+        "--launch-child-share-handles",
+        action="store_true",
+        help=("launch a child inferior.py that shares stdout/stderr/stdio and "
+              "never returns"))
+    parser.add_argument(
+        "--never-return",
+        action="store_true",
+        help="run in an infinite loop, never return")
+    parser.add_argument(
         "--return-code",
         "-r",
         type=int,
@@ -43,7 +53,7 @@ def parse_args(command_line):
     return parser.parse_args(command_line)
 
 
-def maybe_ignore_signals(options, signals):
+def handle_ignore_signals(options, signals):
     """Ignores any signals provided to it.
 
     @param options the command line options parsed by the program.
@@ -61,7 +71,7 @@ def maybe_ignore_signals(options, signal
         signal.signal(signum, signal.SIG_IGN)
 
 
-def maybe_sleep(options, sleep_seconds):
+def handle_sleep(options, sleep_seconds):
     """Sleeps the number of seconds specified, restarting as needed.
 
     @param options the command line options parsed by the program.
@@ -90,7 +100,27 @@ def maybe_sleep(options, sleep_seconds):
             sleep_seconds = sleep_interval.total_seconds()
             if sleep_seconds > 0:
                 time.sleep(sleep_seconds)
-        except:
+        except:  # pylint: disable=bare-except
+            pass
+
+
+def handle_launch_children(options):
+    if options.launch_child_share_handles:
+        # Launch the child, share our file handles.
+        # We won't bother reaping it since it will likely outlive us.
+        subprocess.Popen([sys.executable, __file__, "--never-return"])
+
+
+def handle_never_return(options):
+    if not options.never_return:
+        return
+
+    # Loop forever.
+    while True:
+        try:
+            time.sleep(10)
+        except:  # pylint: disable=bare-except
+            # Ignore
             pass
 
 
@@ -102,8 +132,11 @@ def main(command_line):
     @return the exit value (program return code) for the process.
     """
     options = parse_args(command_line)
-    maybe_ignore_signals(options, options.ignore_signals)
-    maybe_sleep(options, options.sleep_seconds)
+    handle_ignore_signals(options, options.ignore_signals)
+    handle_launch_children(options)
+    handle_sleep(options, options.sleep_seconds)
+    handle_never_return(options)
+
     return options.return_code
 
 if __name__ == "__main__":

Modified: lldb/trunk/test/test_runner/test/process_control_tests.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/test_runner/test/process_control_tests.py?rev=248936&r1=248935&r2=248936&view=diff
==============================================================================
--- lldb/trunk/test/test_runner/test/process_control_tests.py (original)
+++ lldb/trunk/test/test_runner/test/process_control_tests.py Wed Sep 30 15:13:58 2015
@@ -178,7 +178,7 @@ class ProcessControlTimeoutTests(Process
             # ignore soft terminate calls.
             self.inferior_command(
                 ignore_soft_terminate=True,
-                options="--sleep 120"),
+                options="--never-return"),
             "{}s".format(timeout_seconds),
             True)
 
@@ -198,5 +198,35 @@ class ProcessControlTimeoutTests(Process
                  driver.returncode,
                  driver.output))
 
+    def test_inferior_exits_with_live_child_shared_handles(self):
+        """inferior exit detected when inferior children are live with shared
+        stdout/stderr handles.
+        """
+        driver = TestInferiorDriver()
+
+        # Create the inferior (I1), and instruct it to create a child (C1)
+        # that shares the stdout/stderr handles with the inferior.
+        # C1 will then loop forever.
+        driver.run_command_with_timeout(
+            self.inferior_command(
+                options="--launch-child-share-handles --return-code 3"),
+            "5s",
+            False)
+
+        # We should complete without a timetout.  I1 should end
+        # immediately after launching C1.
+        self.assertTrue(
+            driver.completed_event.wait(5),
+            "process failed to complete")
+
+        # Ensure we didn't receive a timeout.
+        self.assertTrue(
+            driver.was_timeout, "inferior should have completed normally")
+
+        self.assertEqual(
+            driver.returncode, 3,
+            "expected inferior process to end with expected returncode")
+
+
 if __name__ == "__main__":
     unittest.main()




More information about the lldb-commits mailing list