tools: reap interrupted RTT symbol lookup

2 files changed, 72 insertions, 21 deletions

diff --git a/test/hil/test/test_hil_rtt.py b/test/hil/test/test_hil_rtt.py
index 04cdf4575..94d0c5eec 100644
--- a/test/hil/test/test_hil_rtt.py
+++ b/test/hil/test/test_hil_rtt.py
@@ -249,6 +249,24 @@ class RttPlatformLifecycle(unittest.TestCase):
         con.close()
         self.assertFalse(os.path.exists(log_name))
 
+    def test_posix_server_log_keeps_automatic_deletion(self):
+        class DoneProc:
+            stdin = None
+            stdout = None
+
+            def poll(self):
+                return 0
+
+        con = hil_util._rtt._SocketRtt()
+        named_temporary_file = tempfile.NamedTemporaryFile
+        with mock.patch.object(hil_util._rtt, 'IS_WINDOWS', False), \
+             mock.patch.object(hil_util._rtt.tempfile, 'NamedTemporaryFile',
+                               wraps=named_temporary_file) as named_log, \
+             mock.patch.object(hil_util._rtt.subprocess, 'Popen', return_value=DoneProc()):
+            con._spawn(['fake-server'])
+        self.assertTrue(named_log.call_args.kwargs['delete'])
+        con.close()
+
     def test_connect_honors_a_stop_request_during_setup(self):
         class FakeProc:
             def poll(self):
@@ -274,6 +292,34 @@ class RttPlatformLifecycle(unittest.TestCase):
                 hil_util._rtt.nm_rtt_addr(str(fake_nm), nm=sys.executable, stop=stop)
         self.assertLess(time.monotonic() - started, 2)
 
+    def test_symbol_lookup_reaps_nm_when_interrupted(self):
+        class InterruptedNm:
+            returncode = None
+
+            def __init__(self):
+                self.terminated = False
+                self.reaped = False
+
+            def communicate(self, timeout=None):
+                if not self.terminated:
+                    raise KeyboardInterrupt
+                self.reaped = True
+                self.returncode = -1
+                return '', ''
+
+            def terminate(self):
+                self.terminated = True
+
+            def kill(self):
+                self.terminated = True
+
+        proc = InterruptedNm()
+        with mock.patch.object(hil_util._rtt.subprocess, 'Popen', return_value=proc), \
+             self.assertRaises(KeyboardInterrupt):
+            hil_util._rtt.nm_rtt_addr('fake.elf', nm='fake-nm', stop=lambda: False)
+        self.assertTrue(proc.terminated)
+        self.assertTrue(proc.reaped)
+
     def test_windows_defaults_to_jlink_exe(self):
         with mock.patch.object(hil_util._rtt, 'IS_WINDOWS', True), \
              mock.patch.dict(os.environ, {}, clear=True):
diff --git a/tools/rtt.py b/tools/rtt.py
index ec9672c52..c3ea86093 100644
--- a/tools/rtt.py
+++ b/tools/rtt.py
@@ -183,27 +183,32 @@ def nm_rtt_addr(elf: str, nm: str = None, stop=None) -> int:
             if stop():
                 raise _StopCapture
             proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
-            deadline = time.monotonic() + 30
-            while True:
-                if stop():
-                    proc.terminate()
+            try:
+                deadline = time.monotonic() + 30
+                while True:
+                    if stop():
+                        raise _StopCapture
+                    remaining = deadline - time.monotonic()
+                    if remaining <= 0:
+                        raise subprocess.TimeoutExpired(cmd, 30)
                     try:
-                        proc.communicate(timeout=2)
+                        stdout, stderr = proc.communicate(timeout=min(0.1, remaining))
+                        r = subprocess.CompletedProcess(cmd, proc.returncode, stdout, stderr)
+                        break
                     except subprocess.TimeoutExpired:
-                        proc.kill()
-                        proc.communicate()
-                    raise _StopCapture
-                remaining = deadline - time.monotonic()
-                if remaining <= 0:
-                    proc.kill()
-                    stdout, stderr = proc.communicate()
-                    raise subprocess.TimeoutExpired(cmd, 30, output=stdout, stderr=stderr)
+                        pass
+            except BaseException:
+                # Signals and stop-file cancellation must not strand nm after main()
+                # returns. Reap it before preserving the original exception.
+                with contextlib.suppress(OSError):
+                    proc.terminate()
                 try:
-                    stdout, stderr = proc.communicate(timeout=min(0.1, remaining))
-                    r = subprocess.CompletedProcess(cmd, proc.returncode, stdout, stderr)
-                    break
+                    proc.communicate(timeout=2)
                 except subprocess.TimeoutExpired:
-                    pass
+                    with contextlib.suppress(OSError):
+                        proc.kill()
+                    proc.communicate()
+                raise
             if stop():
                 raise _StopCapture
     except FileNotFoundError:
@@ -248,10 +253,10 @@ class _SocketRtt:
         # single-threaded server once 64 KiB of log accumulates (openocd at
         # polling_interval 1 against a resetting target fills that in minutes) and
         # the console goes silent with no error; the file also feeds _server_tail
-        # delete=False lets _server_tail() reopen the live file on Windows, where an
-        # auto-delete NamedTemporaryFile otherwise denies the second open. close()
-        # unlinks it explicitly on every platform.
-        self._log = tempfile.NamedTemporaryFile(prefix='rtt-server-', suffix='.log', delete=False)
+        # Windows needs delete=False so _server_tail() can reopen the live file.
+        # POSIX keeps NamedTemporaryFile's automatic close/finalizer cleanup.
+        self._log = tempfile.NamedTemporaryFile(prefix='rtt-server-', suffix='.log',
+                                                delete=not IS_WINDOWS)
         try:
             self._proc = subprocess.Popen(cmd, stdin=stdin, stdout=self._log,
                                           stderr=subprocess.STDOUT, **_popen_group_options())