ci: Linux containment unit tests race asynchronous reaping of killed descendants (flaky on MSHV runners) #320

Summary

NVX microVM tests / Linux / MSHV failed in Validate NVX CLI for #301. The failing test, AdversarialOracleTests.test_pid_journal_failure_cleans_contained_descendants, asserted that the contained processes were gone. They were still exiting or waiting to be reaped. The containment cleanup sends SIGKILL to the whole frozen tree but waits only for its direct child. Linux reparents the other killed processes to a reaper outside the test process, and that reaper collects them asynchronously. The test checks them immediately with os.kill(pid, 0), which also succeeds for a process with a pending SIGKILL and for an unreaped zombie.

The sibling test test_contained_guest_runner_cleans_threaded_detached_child_only fails the same way. A third test that also goes through this cleanup path, test_contained_guest_runner_timeout_kills_descendants, has not failed in CI yet, but it fails locally on dev (see Suggested fix). All three tests reached dev with #289. Runs for three PRs have failed on them, always on Linux/MSHV runners. #301 did not cause the failure, and no process leaked.

Incident

• Workflow: CI run 37082975368, attempt 1, pull_request for [code-deduplication] Deduplicate repository metadata collection #301 (Ready).
• PR head: aaf2ee2697443aa27f85ca7759ffa3d2af43c6af. This is still the current head, so the run was not superseded. The tested merge commit is 62de543293cb0023f178b018c9b289dbb64a9e13, which merges aaf2ee2 into dev at 50d0db5.
• Failed job: NVX microVM tests / Linux / MSHV, step Validate NVX CLI (python3 scripts/test_adversarial.py -v).
• Runner: azure-mshv-1 (self-hosted, linux, mshv, virtual-machine).
• Every other test job passed. These include NVX microVM tests / Linux / KVM, which passed the same test at the same merge commit. Required status check failed only because nvx-microvm-tests-mshv failed.

Decisive output:

FAIL: test_pid_journal_failure_cleans_contained_descendants (__main__.AdversarialOracleTests.test_pid_journal_failure_cleans_contained_descendants)
  File "/opt/nvx-runner/_work/nvx/nvx/scripts/test_adversarial.py", line 1181, in test_pid_journal_failure_cleans_contained_descendants
    self.assertTrue(all(not _process_running(pid) for pid in owned_pids))
AssertionError: False is not true
Ran 67 tests in 8.520s
FAILED (failures=1)

Recurrence

Both tests reached dev when #289 merged (2026-10-02 01:22 UTC). Since then, these assertions have failed four times, always on Linux/MSHV runners:

In CI runs created during that window, 4 of 35 completed Linux/MSHV test jobs failed on these assertions. None of 37 completed Linux/KVM test jobs did. One other Linux/MSHV failure in the window, run 37036696293, was the unrelated #317.

Run 37034965891 shows that the process was not leaked. The assertion found the PID alive. Moments later, os.kill(owned_pid, 9) in the finally block raised ProcessLookupError because the process had already gone away:

    self.assertFalse(_process_running(owned_pid))
AssertionError: True is not false
During handling of the above exception, another exception occurred:
    os.kill(owned_pid, 9)
ProcessLookupError: [Errno 3] No such process

I reran the failed jobs of run 37082975368 twice. Attempt 2 (job 111119403750, azure-mshv-5) failed in the same step on a different test, the network-canary race now tracked in #322. Attempt 3 (job 111120751326, azure-mshv-6) passed, and #301's required check is green.

Root cause

Permalinks point to dev at 50d0db5. These files are byte-identical in the tested merge commit.

1. Containment layout. On Linux, ProcessTreeContainment.command() wraps the command in process_supervisor.py, which runs as a child subreaper (L783-L787). In the test, the process tree is unittest → supervisor → command → a grandchild that calls setsid().
2. Failure path. When record_adversarial_openvmm_pid() raises, InteractiveProcess calls close() (benchmark.py#L1363-L1389). close() calls ProcessTreeContainment.close(), which calls terminate_process_tree(process_group=False) (L804-L814).
3. Only the direct child is awaited. terminate_process_tree() freezes the tree, sends SIGKILL to every PID, and then waits only for the supervisor (L842-L849). The descendant wait loop calls os.waitpid(pid, WNOHANG), but those PIDs are not the caller's children. Each call raises ChildProcessError immediately, and the loop treats that as success (L851-L861). The deadline and the timed out reaping descendant process error therefore never apply. _reap_linux_descendants() handles the same case by polling /proc/<pid> (L495-L500).
4. Reaping happens elsewhere. The supervisor was killed, so it never runs its own reaping step (L555). The kernel reparents the command and the grandchild to a reaper outside the test process, PID 1 on the runner, which collects them later.
5. The check runs immediately. _process_running() is os.kill(pid, 0) (adversarial_executor.py#L269-L276), which succeeds for a process with a pending SIGKILL and for a zombie. The test asserts on it as soon as InteractiveProcess raises (test_adversarial.py#L1181). The sibling test does the same after run_guest_script() returns (L764). Other tests in this file already poll with a 2 s deadline (L871-L873).

The mechanism is not specific to MSHV. Every observed failure ran on an MSHV runner VM, where scheduling or reaping latency evidently reaches the window more often.

Production impact is low. The adversarial executor runs test-microvm through run_bounded_process(contained_by_parent=True). Its outer supervisor adopts reparented descendants and reaps them synchronously before the executor calls _live_openvmm_pids() (L312-L313). A residual window exists only on the timeout path, which already reports the case as not passing. Every process involved has already received SIGKILL, so nothing escapes containment.

Classification: a timing-dependent test defect, exposed by a latent gap in the descendant wait in terminate_process_tree(). It is not a containment leak, and #301 did not cause it. That PR's change to adversarial_executor.py only moves _target_metadata() into common.repository_metadata().

Reproduction

The race reproduces without a hypervisor. I ran it on WSL2 Ubuntu (kernel 6.18.33.2) with Python 3.14.4, 20 logical CPUs, and systemd as PID 1. scripts/ came from the tested merge commit through git archive, and 40 busy loops simulated a contended runner:

git archive 62de543293cb0023f178b018c9b289dbb64a9e13 scripts | tar -x -C /tmp/nvx-62de543
cd /tmp/nvx-62de543
for i in $(seq 1 $(( $(nproc) * 2 ))); do timeout 500 python3 -c 'while True: pass' & done
for i in $(seq 1 60); do
  taskset -c 0 python3 scripts/test_adversarial.py \
    AdversarialOracleTests.test_pid_journal_failure_cleans_contained_descendants >/dev/null 2>&1 || echo FAIL
done | wc -l

When the check reported a PID as running, that PID was either in state R with SIGKILL pending or in state Z as an unreaped zombie. Its parent was always outside the test process. Every such PID disappeared within 7.8 ms, and none of the 600 instrumented iterations left a process running.

Related work

• 5ad0390 ("fix: close Linux containment cleanup races") added the L764 and L1181 tests in network: add bounded egress ranges and CIDR exclusions #289. 0330428 ("test: contain public egress command tree") added the L799 test in the same PR.
• ci: route hosted jobs to isolated Azure runners #318 contains a test-side mitigation, a89cccc ("ci: bound adversarial cleanup test races"). It adds _wait_for_process_exit(), a 2 s poll, before each liveness assertion. The same commit also adds a bounded wait to the loopback-canary test, which is now tracked as a separate product defect in ci: network canary drops connections still queued at close, failing the canary unit test under load #322. Because ci: route hosted jobs to isolated Azure runners #318 is a runner-routing change, these mitigations will not reach dev until it merges.
• ci: network canary drops connections still queued at close, failing the canary unit test under load #322 covers the other flaky test in this CI step, test_filesystem_and_network_canaries_detect_policy_violations. Its root cause and suggested fix are separate from this issue.
• Parent feature: Add Copilot-driven adversarial stress and security testing #102.

Suggested fix

Make terminate_process_tree() guarantee what its descendant loop already attempts: when it returns, every process it killed has exited. Today os.waitpid() raises ChildProcessError for any descendant that is not the caller's child, and the loop treats that as done. With the fix, the loop keeps polling such a descendant until every thread in /proc/<pid>/task is a zombie (Z) or dead (X), or until the PID disappears. The existing deadline still applies, and it still raises ScriptError on timeout.

By the time a process is a zombie, it has already released its memory and file descriptors, so the fix does not depend on when an outside reaper collects it. That matters because a descendant with SIGKILL pending can briefly keep running while it still holds resources such as TAP devices or hypervisor handles. The fix also treats ProcessLookupError (ESRCH) like FileNotFoundError when it reads /proc/<pid>/task/<tid>/stat. A reaper can remove the task between open() and read(), and the new polling would otherwise surface that as a cleanup error. On Linux 6.18, such a read returned ESRCH.

The tests change in three ways:

• The three liveness assertions that follow this cleanup (L764, L799, L1181) use a new _process_exited() helper. On Linux it treats zombies as exited. On other platforms it keeps the existing _process_running() behavior.
• Both finally blocks ignore ProcessLookupError from os.kill, so the cleanup can no longer hide the assertion as it did in run 37034965891.
• Two mock tests pin the new behavior. The loop polls until a reparented descendant exits, and it raises when that descendant never does.

I considered two alternatives:

• The test-only bounded wait from a89cccc makes the tests pass. However, it leaves the descendant wait in terminate_process_tree() ineffective, which matters to production callers.
• Changing only the tests to treat zombies as exited is not enough. In the "zombie-aware test only" row below, the L1181 test still failed 9 of 20 times, because the killed processes were still running when the check ran.

The patch applies cleanly to dev at d54b8c3 with git apply, and also on its own.

diff --git a/scripts/nvx_tools/adversarial_oracles.py b/scripts/nvx_tools/adversarial_oracles.py
index 81e276a..cd2d1ed 100644
--- a/scripts/nvx_tools/adversarial_oracles.py
+++ b/scripts/nvx_tools/adversarial_oracles.py
@@ -31,6 +31,7 @@ PROCESS_CLEANUP_GRACE_SECONDS = (
     PROCESS_TERMINATION_WAIT_SECONDS * 2 + PROCESS_READER_JOIN_SECONDS * 2
 )
 _LINUX_CHILD_SUBREAPER = 36
+_LINUX_EXITED_TASK_STATES = frozenset({"Z", "X", "x"})
 _WINDOWS_CREATE_SUSPENDED = 0x00000004
 
 
@@ -398,7 +399,7 @@ def _linux_task_states(pid: int) -> tuple[tuple[int, str], ...]:
         stat_path = task_path / "stat"
         try:
             stat = stat_path.read_text()
-        except FileNotFoundError:
+        except (FileNotFoundError, ProcessLookupError):
             continue
         except OSError as error:
             raise ScriptError(
@@ -416,6 +417,13 @@ def _linux_task_states(pid: int) -> tuple[tuple[int, str], ...]:
     return tuple(sorted(states))
 
 
+def _linux_process_exited(pid: int) -> bool:
+    """Return whether every thread has exited, even if the PID awaits reaping."""
+    return all(
+        state in _LINUX_EXITED_TASK_STATES for _, state in _linux_task_states(pid)
+    )
+
+
 def _wait_linux_process_quiescent(pid: int, deadline: float) -> bool:
     quiescent_task_ids: tuple[int, ...] | None = None
     while True:
@@ -853,9 +861,13 @@ def terminate_process_tree(
                 try:
                     reaped, _ = os.waitpid(pid, os.WNOHANG)
                 except ChildProcessError:
-                    break
-                if reaped == pid:
-                    break
+                    # Another process reaps this descendant, so wait only until
+                    # all of its threads have exited.
+                    if _linux_process_exited(pid):
+                        break
+                else:
+                    if reaped == pid:
+                        break
                 if time.monotonic() >= deadline:
                     raise ScriptError(f"timed out reaping descendant process {pid}")
                 time.sleep(0.01)
diff --git a/scripts/test_adversarial.py b/scripts/test_adversarial.py
index 83c7730..48750c5 100644
--- a/scripts/test_adversarial.py
+++ b/scripts/test_adversarial.py
@@ -3,6 +3,7 @@ from __future__ import annotations
 
 import argparse
 import base64
+import contextlib
 import ctypes
 import hashlib
 import json
@@ -60,10 +61,12 @@ from nvx_tools.adversarial_oracles import (
     BoundedProcessResult,
     OracleSession,
     _freeze_linux_process_tree,
+    _linux_process_exited,
     _reap_linux_children,
     _reap_linux_descendants,
     _WindowsJob,
     run_bounded_process,
+    terminate_process_tree,
 )
 from nvx_tools.benchmark import InteractiveProcess, run_guest_script
 from nvx_tools.build_constants import (
@@ -314,6 +317,12 @@ def _fake_executor_client_type(
     return FakeExecutorClient
 
 
+def _process_exited(pid: int) -> bool:
+    if sys.platform.startswith("linux"):
+        return _linux_process_exited(pid)
+    return not _process_running(pid)
+
+
 class AdversarialBrokerTests(unittest.TestCase):
     def test_campaign_catalogs_are_strict_and_nonempty(self) -> None:
         for campaign in CAMPAIGNS:
@@ -675,6 +684,64 @@ class AdversarialOracleTests(unittest.TestCase):
 
         self.assertEqual(events, ["observe", "observe", "observe", "enumerate"])
 
+    @unittest.skipUnless(sys.platform.startswith("linux"), "Linux process ownership")
+    def test_process_tree_cleanup_waits_for_reparented_descendants(self) -> None:
+        process = MagicMock()
+        process.pid = 100
+        process.poll.return_value = None
+        with (
+            patch(
+                "nvx_tools.adversarial_oracles._freeze_linux_process_tree",
+                return_value=[300, 200, 100],
+            ),
+            patch("nvx_tools.adversarial_oracles.os.kill"),
+            patch(
+                "nvx_tools.adversarial_oracles.os.waitpid",
+                side_effect=ChildProcessError,
+            ),
+            patch(
+                "nvx_tools.adversarial_oracles._linux_process_exited",
+                side_effect=(False, True, True),
+            ) as exited,
+            patch("nvx_tools.adversarial_oracles.time.sleep"),
+        ):
+            terminate_process_tree(process, process_group=False)
+
+        self.assertEqual(exited.call_args_list, [call(300), call(300), call(200)])
+
+    @unittest.skipUnless(sys.platform.startswith("linux"), "Linux process ownership")
+    def test_process_tree_cleanup_times_out_on_running_reparented_descendant(
+        self,
+    ) -> None:
+        process = MagicMock()
+        process.pid = 100
+        process.poll.return_value = None
+        with (
+            patch(
+                "nvx_tools.adversarial_oracles._freeze_linux_process_tree",
+                return_value=[200, 100],
+            ),
+            patch("nvx_tools.adversarial_oracles.os.kill"),
+            patch(
+                "nvx_tools.adversarial_oracles.os.waitpid",
+                side_effect=ChildProcessError,
+            ),
+            patch(
+                "nvx_tools.adversarial_oracles._linux_process_exited",
+                return_value=False,
+            ),
+            patch(
+                "nvx_tools.adversarial_oracles.time.monotonic",
+                side_effect=(100.0, 106.0),
+            ),
+            patch("nvx_tools.adversarial_oracles.time.sleep"),
+            self.assertRaisesRegex(
+                ScriptError,
+                "timed out reaping descendant process 200",
+            ),
+        ):
+            terminate_process_tree(process, process_group=False)
+
     @unittest.skipUnless(sys.platform.startswith("linux"), "Linux process ownership")
     def test_contained_guest_runner_cleans_threaded_detached_child_only(self) -> None:
         for completion_marker in (b"SYNTHETIC-COMPLETE", b"NEVER-COMPLETE"):
@@ -761,12 +828,13 @@ class AdversarialOracleTests(unittest.TestCase):
                     self.assertEqual(len(unrelated), 1)
                     self.assertIsNone(unrelated[0].poll())
                     owned_pid = int(owned_pid_path.read_text(encoding="utf-8"))
-                    self.assertFalse(_process_running(owned_pid))
+                    self.assertTrue(_process_exited(owned_pid))
                 finally:
                     if owned_pid_path.exists():
                         owned_pid = int(owned_pid_path.read_text(encoding="utf-8"))
                         if _process_running(owned_pid):
-                            os.kill(owned_pid, 9)
+                            with contextlib.suppress(ProcessLookupError):
+                                os.kill(owned_pid, 9)
                     if unrelated:
                         unrelated[0].kill()
                         unrelated[0].wait()
@@ -796,7 +864,7 @@ class AdversarialOracleTests(unittest.TestCase):
                     contain_process_tree=True,
                 )
             child_pid = int(child_pid_path.read_text(encoding="utf-8"))
-        self.assertFalse(_process_running(child_pid))
+        self.assertTrue(_process_exited(child_pid))
 
     def test_contained_guest_runner_normal_exit_kills_descendants(self) -> None:
         with tempfile.TemporaryDirectory() as temporary:
@@ -1178,12 +1246,13 @@ class AdversarialOracleTests(unittest.TestCase):
                     int(command_pid_path.read_text(encoding="utf-8")),
                     int(descendant_pid_path.read_text(encoding="utf-8")),
                 ]
-                self.assertTrue(all(not _process_running(pid) for pid in owned_pids))
+                self.assertTrue(all(_process_exited(pid) for pid in owned_pids))
                 self.assertIsNone(unrelated.poll())
             finally:
                 for pid in owned_pids:
                     if _process_running(pid):
-                        os.kill(pid, 9)
+                        with contextlib.suppress(ProcessLookupError):
+                            os.kill(pid, 9)
                 unrelated.kill()
                 unrelated.wait()
 

Validation of the suggested patch

I validated the patch in scratch copies of dev at d54b8c3, outside any working tree, on WSL2 Ubuntu (kernel 6.18, Python 3.14, 20 CPUs) and on Windows. I did not commit or push anything. For the contention runs, the test and three busy loops were all pinned to CPU 0.

The suggested fix for #322 adds a different name to the same nvx_tools.adversarial_oracles import list in scripts/test_adversarial.py. Whichever patch lands second therefore needs a one-line rebase there, or can be applied with git apply -C1. I applied the two patches in both orders, and each time the result was identical to the combined version validated above.

Acceptance criteria

• terminate_process_tree() returns only after every descendant it killed has exited. Otherwise it raises ScriptError when the deadline expires.
• On Linux under CPU contention, the L764, L799, and L1181 tests each pass at least 100 consecutive iterations.
• A descendant that cleanup does not kill still causes the tests to fail.
• The Linux/MSHV Validate NVX CLI steps no longer fail on these assertions.