From 755c9eabe778cfab61cf07bd21e1bab28e816ffe Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 05:47:16 +0000 Subject: [PATCH] =?UTF-8?q?test(core):=20=E8=AE=A9=20#4875=20=E4=B8=8D?= =?UTF-8?q?=E5=8F=98=E9=87=8F=E9=92=89=E4=BD=8F=20monitor=20=E8=87=AA?= =?UTF-8?q?=E5=B7=B1=E7=9A=84=20guard,=E8=80=8C=E4=B8=8D=E6=98=AF=E8=BF=9B?= =?UTF-8?q?=E7=A8=8B=E5=85=A8=E5=B1=80=20ref'd=20timer=20=E8=AE=A1?= =?UTF-8?q?=E6=95=B0=20(#6329)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `leaves no ref'd timer behind when the health check wins the race` 此前用 `process.getActiveResourcesInfo()` 的进程全局 ref'd Timeout 计数做「await 前后 比对」。该计数是全进程共享的:vitest runner 自己就在同一个事件循环上挂着一个 **没有 unref 的 100ms** 定时器(`throttle(sendTasksUpdate, 100)`,@vitest/runner), 而它的 per-test 超时守卫反而显式 `timer.unref?.()`、根本不计入。于是合并队列全量并发 把两次读数之间的窗口拉长到 100ms 以上(失败那次实测 105ms)时,runner 的节流定时器 在窗口内到期,计数凭空少 1,断言读到 `expected +0 to be 1`——与 monitor 无关。 改为:用一次性的 `setTimeout` 记录取回 monitor 本轮真正armed 的 guard 句柄(按配置的 timeout 值与循环上其它定时器区分),随后所有读数都放在**同一个同步回合**里相邻取, 两条语句之间不可能有任何定时器回调运行,因此差值只可能是 monitor 自己造成的。 断言强度不变(反向验证):去掉 finally 里的 clearTimeout ⇒ 本用例与 fake-timer 同伴用例双红;把 clearTimeout 换成 arm 时 unref ⇒ 本用例绿、fake-timer 同伴红, 与改动前的分工完全一致。新增 `expect(guards).toHaveLength(1)` 防止「什么都没测到」 的空绿。生产面零改动。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We --- packages/core/src/health-monitor.test.ts | 88 +++++++++++++++++++++--- 1 file changed, 78 insertions(+), 10 deletions(-) diff --git a/packages/core/src/health-monitor.test.ts b/packages/core/src/health-monitor.test.ts index a4ddd28617..a69fd0e4dc 100644 --- a/packages/core/src/health-monitor.test.ts +++ b/packages/core/src/health-monitor.test.ts @@ -120,28 +120,96 @@ describe('PluginHealthMonitor', () => { * Ref'd `Timeout` handles — `getActiveResourcesInfo()` reports only * resources currently keeping the event loop alive, which is exactly the * property that made `os migrate` idle ~120s in #4813. + * + * The count is *process*-global, so it is only ever read here in + * synchronously adjacent pairs (see below). Comparing two reads separated + * by an `await` is what made this suite flaky in the merge queue (#6329): + * the runner shares this loop and keeps a **non-unref'd 100ms** timer on it + * (`throttle(sendTasksUpdate, 100)` in `@vitest/runner`), so once the + * window between the reads stretched past 100ms under full concurrent load + * — the failing run measured 105ms — that timer fired inside the window and + * the count fell by one for a reason that had nothing to do with the + * monitor. Between two adjacent synchronous statements no timer callback + * can run at all, so a difference measured that way is the monitor's doing + * and nobody else's. */ const refdTimers = () => process.getActiveResourcesInfo().filter((r) => r === 'Timeout').length; + /** + * Run `body` while recording the `Timeout` handles `setTimeout` hands out, + * and return those armed with `delay` — the monitor's health-check guards, + * told apart from every other timer on the shared loop by the very timeout + * they were configured with. + * + * Holding the handles is what lets the assertion below name the guard + * instead of counting the world. It records where the guard came from; what + * it then asserts is still the observable consequence — whether that handle + * is keeping the event loop alive — never that `clearTimeout` was called. + */ + const recordingGuards = async ( + delay: number, + body: () => Promise + ): Promise => { + const guards: NodeJS.Timeout[] = []; + const real = globalThis.setTimeout; + const recording = ((...args: Parameters) => { + const handle = real(...args); + if (args[1] === delay) guards.push(handle); + return handle; + }) as typeof globalThis.setTimeout; + Object.assign(recording, real); + + globalThis.setTimeout = recording; + try { + await body(); + } finally { + globalThis.setTimeout = real; + } + return guards; + }; + it("leaves no ref'd timer behind when the health check wins the race", async () => { const calls = { count: 0 }; - monitor.registerPlugin('guarded-plugin', guardedConfig()); + const config = guardedConfig(); + monitor.registerPlugin('guarded-plugin', config); - const before = refdTimers(); - monitor.startMonitoring('guarded-plugin', healthyPlugin(calls)); + const guards = await recordingGuards(config.timeout, async () => { + monitor.startMonitoring('guarded-plugin', healthyPlugin(calls)); - // The initial check runs immediately; wait for its report to land. - await vi.waitFor(() => { - expect(monitor.getHealthReport('guarded-plugin')).toBeDefined(); + // The initial check runs immediately; wait for its report to land. + await vi.waitFor(() => { + expect(monitor.getHealthReport('guarded-plugin')).toBeDefined(); + }); }); - // Drop the monitoring interval — whatever is left is the guard's doing. - monitor.stopMonitoring('guarded-plugin'); - expect(calls.count).toBe(1); expect(monitor.getHealthStatus('guarded-plugin')).toBe('healthy'); - expect(refdTimers()).toBe(before); + + // The round armed exactly one guard. Without this the reclaim below would + // be vacuously green — a difference of zero because nothing was measured, + // rather than because nothing was left behind. + expect(guards).toHaveLength(1); + + // Everything from here to the last assertion runs in one uninterrupted + // synchronous turn, so each difference is attributable. + const whileMonitoring = refdTimers(); + + // Drop the monitoring interval — whatever is left is the guard's doing. + monitor.stopMonitoring('guarded-plugin'); + const afterStop = refdTimers(); + + // The interval was pinning the loop and is now reclaimed. This also keeps + // the instrument honest: `refdTimers()` demonstrably observes *this* + // monitor's timers on *this* loop, so the guard's zero below is a real + // reading and not a blind one. + expect(whileMonitoring - afterStop).toBe(1); + + // The guard is not pinning the loop: reclaiming it a second time is a + // no-op. Had it outlived the race it would still be armed and ref'd, and + // this reclaim would drop the count by one. + for (const guard of guards) clearTimeout(guard); + expect(refdTimers()).toBe(afterStop); }); it('still reports the timeout when the check never answers', async () => {