Unclearly exit if terminating takes too long

This could happen if someone catches `BaseException`, or due to another issue with cleanup.

Also add some specific test cases to catch the specific cases, which can be added to over time.

sbot/timeout.py

@@ -9,6 +9,8 @@
 
 logger = logging.getLogger(__name__)
 
+TIMEOUT_MESSAGE = "Timeout expired: Game Over!"
+
 
 def timeout_handler(signal_type: int, stack_frame: Optional[FrameType]) -> None:
     """
@@ -17,12 +19,21 @@ def timeout_handler(signal_type: int, stack_frame: Optional[FrameType]) -> None:
     This function is called when the timeout expires and will stop the robot's main process.
     In order for this to work, any threads that are created must be daemons.
 
+    If the process doesn't terminate clearly (perhaps because the exception was caught),
+    exit less cleanly.
+
     NOTE: This function is not called on Windows.
 
     :param signal_type: The sginal that triggered this handler
     :param stack_frame: The stack frame at the time of the signal
     """
-    logger.info("Timeout expired: Game Over!")
+    logger.info(TIMEOUT_MESSAGE)
+
+    # If the process doesn't terminate in a given time, exit less cleanly
+    signal.signal(signal.SIGALRM, signal.SIG_DFL)
+    signal.alarm(2)
+
+    # Exit cleanly
     exit(0)
 
 
@@ -35,7 +46,7 @@ def win_timeout_handler() -> None:
 
     NOTE: This function is only called on Windows.
     """
-    logger.info("Timeout expired: Game Over!")
+    logger.info(TIMEOUT_MESSAGE)
     os.kill(os.getpid(), signal.SIGTERM)
 
 

tests/test_data/timeout_scripts/catch-base-exception.py

@@ -0,0 +1,10 @@
+from sbot.timeout import kill_after_delay
+import time
+
+kill_after_delay(2)
+
+while True:
+    try:
+        time.sleep(10)
+    except BaseException:
+        pass

tests/test_data/timeout_scripts/catch-exception.py

@@ -0,0 +1,10 @@
+from sbot.timeout import kill_after_delay
+import time
+
+kill_after_delay(2)
+
+while True:
+    try:
+        time.sleep(10)
+    except Exception:
+        pass

tests/test_data/timeout_scripts/hot-loop.py

@@ -0,0 +1,7 @@
+from sbot.timeout import kill_after_delay
+
+kill_after_delay(2)
+
+# This is a bad idea
+while True:
+    pass

tests/test_data/timeout_scripts/sleep.py

@@ -0,0 +1,6 @@
+from sbot.timeout import kill_after_delay
+import time
+
+kill_after_delay(2)
+
+time.sleep(10)

tests/test_data/timeout_scripts/try-finally.py

@@ -0,0 +1,11 @@
+from sbot.timeout import kill_after_delay
+import time
+
+kill_after_delay(2)
+
+while True:
+    try:
+        time.sleep(10)
+    finally:
+        # Imagine we wanted to do something else here
+        pass

tests/test_timeout.py

@@ -2,6 +2,7 @@
 from time import sleep, time
 
 import os
+from pathlib import Path
 import signal
 import subprocess
 import sys
@@ -10,13 +11,18 @@
 
 from sbot.timeout import kill_after_delay
 
+TEST_FILES = list((Path(__file__).parent / 'test_data/timeout_scripts').iterdir())
+
 @pytest.mark.skipif(sys.platform == "win32", reason="does not run on Windows")
 def test_kill_after_delay() -> None:
     """Test that the process is killed within the time."""
     with pytest.raises(SystemExit):
         kill_after_delay(2)
         sleep(3)
 
+    # Clear the set alarm
+    signal.alarm(0)
+
 
 @pytest.mark.skipif(sys.platform != "win32", reason="only runs on Windows")
 def test_kill_after_delay_windows(monkeypatch) -> None:
@@ -34,20 +40,26 @@ def test_kill_after_delay_windows(monkeypatch) -> None:
     kill.assert_called_once_with(pid, signal.SIGTERM)
 
 
-def test_kill_after_delay_e2e() -> None:
+@pytest.mark.parametrize(
+    "test_file",
+    [str(f) for f in TEST_FILES],
+    ids=[f.name for f in TEST_FILES]
+)
+def test_kill_after_delay_e2e(test_file: Path) -> None:
     start_time = time()
     child = subprocess.Popen([
         sys.executable,
-        "-c",
-        'from sbot.timeout import kill_after_delay; import time; kill_after_delay(2); time.sleep(10)',
+        str(test_file),
     ], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
 
-    child.wait(timeout=5)
+    child.wait(timeout=6)
+    run_time = time() - start_time
 
-    assert time() - start_time == pytest.approx(2, rel=1)
+    assert 2 < run_time < 6
 
     if sys.platform == "win32":
         # Windows terminates uncleanly
         assert child.returncode == signal.SIGTERM
     else:
-        assert child.returncode == 0
+        # Either the process was killed cleanly, or the fallback did
+        assert child.returncode in [0, -signal.SIGALRM]