Skip to content

Commit 2f386d7

Browse files
baozhoutaoclaude
andauthored
test(core): 让 #4875 不变量钉住 monitor 自己的 guard,而不是进程全局 ref'd timer 计数 (#6329) (#6582)
`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)` 防止「什么都没测到」 的空绿。生产面零改动。 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We Co-authored-by: Claude <noreply@anthropic.com>
1 parent e9b5265 commit 2f386d7

1 file changed

Lines changed: 78 additions & 10 deletions

File tree

packages/core/src/health-monitor.test.ts

Lines changed: 78 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,28 +120,96 @@ describe('PluginHealthMonitor', () => {
120120
* Ref'd `Timeout` handles — `getActiveResourcesInfo()` reports only
121121
* resources currently keeping the event loop alive, which is exactly the
122122
* property that made `os migrate` idle ~120s in #4813.
123+
*
124+
* The count is *process*-global, so it is only ever read here in
125+
* synchronously adjacent pairs (see below). Comparing two reads separated
126+
* by an `await` is what made this suite flaky in the merge queue (#6329):
127+
* the runner shares this loop and keeps a **non-unref'd 100ms** timer on it
128+
* (`throttle(sendTasksUpdate, 100)` in `@vitest/runner`), so once the
129+
* window between the reads stretched past 100ms under full concurrent load
130+
* — the failing run measured 105ms — that timer fired inside the window and
131+
* the count fell by one for a reason that had nothing to do with the
132+
* monitor. Between two adjacent synchronous statements no timer callback
133+
* can run at all, so a difference measured that way is the monitor's doing
134+
* and nobody else's.
123135
*/
124136
const refdTimers = () =>
125137
process.getActiveResourcesInfo().filter((r) => r === 'Timeout').length;
126138

139+
/**
140+
* Run `body` while recording the `Timeout` handles `setTimeout` hands out,
141+
* and return those armed with `delay` — the monitor's health-check guards,
142+
* told apart from every other timer on the shared loop by the very timeout
143+
* they were configured with.
144+
*
145+
* Holding the handles is what lets the assertion below name the guard
146+
* instead of counting the world. It records where the guard came from; what
147+
* it then asserts is still the observable consequence — whether that handle
148+
* is keeping the event loop alive — never that `clearTimeout` was called.
149+
*/
150+
const recordingGuards = async (
151+
delay: number,
152+
body: () => Promise<void>
153+
): Promise<NodeJS.Timeout[]> => {
154+
const guards: NodeJS.Timeout[] = [];
155+
const real = globalThis.setTimeout;
156+
const recording = ((...args: Parameters<typeof globalThis.setTimeout>) => {
157+
const handle = real(...args);
158+
if (args[1] === delay) guards.push(handle);
159+
return handle;
160+
}) as typeof globalThis.setTimeout;
161+
Object.assign(recording, real);
162+
163+
globalThis.setTimeout = recording;
164+
try {
165+
await body();
166+
} finally {
167+
globalThis.setTimeout = real;
168+
}
169+
return guards;
170+
};
171+
127172
it("leaves no ref'd timer behind when the health check wins the race", async () => {
128173
const calls = { count: 0 };
129-
monitor.registerPlugin('guarded-plugin', guardedConfig());
174+
const config = guardedConfig();
175+
monitor.registerPlugin('guarded-plugin', config);
130176

131-
const before = refdTimers();
132-
monitor.startMonitoring('guarded-plugin', healthyPlugin(calls));
177+
const guards = await recordingGuards(config.timeout, async () => {
178+
monitor.startMonitoring('guarded-plugin', healthyPlugin(calls));
133179

134-
// The initial check runs immediately; wait for its report to land.
135-
await vi.waitFor(() => {
136-
expect(monitor.getHealthReport('guarded-plugin')).toBeDefined();
180+
// The initial check runs immediately; wait for its report to land.
181+
await vi.waitFor(() => {
182+
expect(monitor.getHealthReport('guarded-plugin')).toBeDefined();
183+
});
137184
});
138185

139-
// Drop the monitoring interval — whatever is left is the guard's doing.
140-
monitor.stopMonitoring('guarded-plugin');
141-
142186
expect(calls.count).toBe(1);
143187
expect(monitor.getHealthStatus('guarded-plugin')).toBe('healthy');
144-
expect(refdTimers()).toBe(before);
188+
189+
// The round armed exactly one guard. Without this the reclaim below would
190+
// be vacuously green — a difference of zero because nothing was measured,
191+
// rather than because nothing was left behind.
192+
expect(guards).toHaveLength(1);
193+
194+
// Everything from here to the last assertion runs in one uninterrupted
195+
// synchronous turn, so each difference is attributable.
196+
const whileMonitoring = refdTimers();
197+
198+
// Drop the monitoring interval — whatever is left is the guard's doing.
199+
monitor.stopMonitoring('guarded-plugin');
200+
const afterStop = refdTimers();
201+
202+
// The interval was pinning the loop and is now reclaimed. This also keeps
203+
// the instrument honest: `refdTimers()` demonstrably observes *this*
204+
// monitor's timers on *this* loop, so the guard's zero below is a real
205+
// reading and not a blind one.
206+
expect(whileMonitoring - afterStop).toBe(1);
207+
208+
// The guard is not pinning the loop: reclaiming it a second time is a
209+
// no-op. Had it outlived the race it would still be armed and ref'd, and
210+
// this reclaim would drop the count by one.
211+
for (const guard of guards) clearTimeout(guard);
212+
expect(refdTimers()).toBe(afterStop);
145213
});
146214

147215
it('still reports the timeout when the check never answers', async () => {

0 commit comments

Comments
 (0)