From f6bf2011a0922f4a78f4ac9f7e7be8d514835df1 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Tue, 15 Sep 2026 10:07:23 +0200 Subject: [PATCH] fix(registry-mock): wait for the reaped registry child to leave the process table (#14917) `ChildCleanup::assert_reaped` sampled the process table once, immediately after the kill. On Windows a terminated process stays listed until its last handle closes, so the PID can outlive the `wait()` that reaped it and the assertion fires on a child that is already gone. This made `successful_startup_retains_owner_and_reuse_does_not_stop_registry` flaky on the Windows runner; it is the one test of the three that has no await between dropping the instance and asserting. Poll for up to ten seconds instead of sampling once. A child that is genuinely left running still fails the assertion, verified by removing the kill from `MockInstance::drop` and watching the test fail after the full budget. Claude-Session: https://claude.ai/code/session_01GP1WscFutimgTWpePgVSV7 Co-authored-by: Claude Opus 5 (1M context) --- .../registry-mock/src/mock_instance/tests.rs | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/pnpm/tasks/registry-mock/src/mock_instance/tests.rs b/pnpm/tasks/registry-mock/src/mock_instance/tests.rs index 514e362e9f..71cc9549ad 100644 --- a/pnpm/tasks/registry-mock/src/mock_instance/tests.rs +++ b/pnpm/tasks/registry-mock/src/mock_instance/tests.rs @@ -6,7 +6,7 @@ use sysinfo::{Pid, ProcessRefreshKind, RefreshKind, Signal, System, UpdateKind}; use tokio::{ net::{TcpListener, TcpStream}, task::JoinHandle, - time::{Duration, timeout}, + time::{Duration, Instant, sleep, timeout}, }; fn options(client: &Client, port: u16) -> MockInstanceOptions<'_> { @@ -28,6 +28,10 @@ fn processes() -> System { ) } +/// How long [`ChildCleanup::assert_reaped`] waits for the process table to +/// catch up before it calls the child abandoned. +const REAP_TIMEOUT: Duration = Duration::from_secs(10); + /// Last-resort kill for the registry child a test spawned, so a failing /// assertion never leaves the process behind. struct ChildCleanup(Option); @@ -35,12 +39,18 @@ struct ChildCleanup(Option); impl ChildCleanup { /// Disarms once the child is confirmed gone, because the OS is free to /// hand that PID to an unrelated process afterwards. - fn assert_reaped(mut self) { + /// + /// Windows keeps a terminated process listed until its last handle + /// closes, so the PID can outlive the `wait()` that reaped it. Polling + /// still fails a child that is genuinely left running, it just does not + /// race the process table. + async fn assert_reaped(mut self) { let pid = self.0.expect("cleanup is armed"); - assert!( - processes().process(pid).is_none(), - "registry child {pid} must be stopped and reaped", - ); + let deadline = Instant::now() + REAP_TIMEOUT; + while processes().process(pid).is_some() { + assert!(Instant::now() < deadline, "registry child {pid} must be stopped and reaped"); + sleep(Duration::from_millis(50)).await; + } self.0 = None; } } @@ -90,7 +100,7 @@ async fn cancelled_startup_reaps_child() { startup.abort(); let error = startup.await.unwrap_err(); assert!(error.is_cancelled(), "aborting startup cancels it: {error}"); - child.assert_reaped(); + child.assert_reaped().await; } #[tokio::test] @@ -103,7 +113,7 @@ async fn failed_readiness_check_reaps_child() { .expect("closed connection must fail the readiness check") .unwrap_err(); assert!(error.is_panic(), "readiness failures panic: {error}"); - child.assert_reaped(); + child.assert_reaped().await; } #[tokio::test] @@ -118,5 +128,5 @@ async fn successful_startup_retains_owner_and_reuse_does_not_stop_registry() { assert!(registry.spawn_if_necessary().await.is_none(), "a ready registry is not respawned"); assert!(registry.is_registry_ready().await, "reuse must leave the registry serving"); drop(instance); - child.assert_reaped(); + child.assert_reaped().await; }