fix(service-automation,core): 超时守卫在 race 落定时被清除 —— #4813 / #4875 的清仓 (#4952) - #4982
Merged
Merged
Conversation
…imeout guards when the race settles (#4952) Two remaining production instances of the leak #4813 (PR #4874, kernel init/start) and #4875 (PR #4950, periodic health checks) already fixed: a timeout guard armed and then abandoned. When the guarded side won the race, its `setTimeout` was neither cleared nor unref'd, so it stayed ref'd and pinned the event loop for the whole timeout budget. - `AutomationEngine.executeWithTimeout()` — the widest of the three: one guard per flow node that declares `timeoutMs`, per run, so the orphan count scales with flow size x trigger frequency. A one-shot process (`os` CLI running a flow) idles for the longest armed budget after its work is done. - `HotReloadManager.reloadPlugin()` — the plugin `destroy()` shutdown guard, byte-for-byte the shape #4813 fixed. Both follow #4874 / #4950 exactly — a private helper plus `try { return await Promise.race([...]) } finally { clearTimeout(guard) }`. No new variant, no `unref()`. hot-reload's helper widens its input to `T | PromiseLike<T>` (the Plugin contract permits a synchronous `destroy()`); engine's does not (`NodeExecutor.execute` is declared Promise-returning). `unref()` would stop the pinning but also stop the guard being a guard: if the guarded side never settles and nothing else keeps the loop alive, Node exits before the timer fires and the timeout is never reported. Regression tests therefore assert both directions per site, following #4950: no ref'd timer left under real timers, no accumulation across rounds under fake timers (that count still sees an unref'd timer, so it catches an unref()-style fake fix), and the timeout still reported when the guarded side genuinely hangs. Timeout durations are untouched — the defect was never the duration. B1 (kernel.ts `shutdown()`) and B2 (objectql `checkDriversHealth()`) are deliberately out of scope per the PM ruling on #4952: both are documented deliberate `unref()` semantics. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX
…eout-guard-cleanup
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 26 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4952
这是什么
#4813(PR #4874,
kernel.ts的 init/start)与 #4875(PR #4950,health-monitor.ts的健康检查)修掉的是同一种漏法:守卫 armed 之后就被扔掉 —— 被守护的一方赢下 race 之后,那根setTimeout既没clearTimeout也没unref(),带着 ref 一直把事件循环钉满整个超时预算。本 PR 清掉 issue 里 A 类(真·漏)剩下的两处生产实例。A1
AutomationEngine.executeWithTimeout()(packages/services/service-automation/src/engine.ts)三处里量级最大的一处:调用点在每个流程节点上,每个声明了
timeoutMs的节点各一根守卫,孤儿数随流程节点数 × 触发频率线性增长;并且与 #4875 一样会把一次性进程(osCLI 跑到 flow 的路径)按最长的那根守卫钉住到超时才退出 —— 活早干完了。A2
HotReloadManager.reloadPlugin()(packages/core/src/hot-reload.ts)插件
destroy()的 shutdown 守卫,与 #4813 修掉的两处一字不差。热重载路径今天调用不多,但一次毫秒级完成的重载照样把循环钉满shutdownTimeout。修法:与 #4874 / #4950 同形,不新造变体
两处都是私有 helper +
try { return await Promise.race([...]) } finally { clearTimeout(guard) },注释风格沿用那两处(含「为什么不是unref()」那段)。两个 helper 的差别只有一处,且是有理由的:hot-reload.ts的raceShutdownTimeout< T >把入参放宽到T | PromiseLike< T >—— Plugin 契约允许同步destroy(): void(packages/core/src/types.ts),与 kernel / health-monitor 两处放宽的理由相同;engine.ts的executeWithTimeout不放宽 ——NodeExecutor.execute声明返回Promise< NodeExecutionResult >。没有做成跨文件共享 helper。 issue 的派发单允许在
packages/core内部考虑与health-monitor.ts的raceCheckTimeout共用。结论是不共用:本轮的硬约束是kernel.ts零改动,所以一个共享 helper 只会被hot-reload.ts一个调用点使用,而kernel.ts/health-monitor.ts各自保留副本 —— 那是「三份实现里有一份长得不一样」,比现状更难读。每文件一个同形 local helper 正是这两个 PR 已经确立的惯例(kernel、health-monitor 各有一份),本 PR 照此执行;真要收敛应当是一次单独的、把三处一起搬走的重构。超时时长(
timeoutMs/shutdownTimeout)一个都没动 —— 问题从来不在时长,而在没人回收。不在本 PR 范围内(维护者/PM 在 issue 上的裁定)
issue 的 B 类两处刻意不动,按 #4952 上的 PM 裁定它们是有意为之的既定语义:
packages/core/src/kernel.ts:430shutdown()—— 显式unref():进程正在退出,吞掉一次超时是可接受的;packages/objectql/src/engine.ts:2971checkDriversHealth()—— 显式unref(),代码里的注释(Never hold the event loop open for a probe)已经把这个选择记录在案。除非维护者在 issue 上推翻该裁定,这两处保持原样;本 PR 对
kernel.ts与objectql/engine.ts零改动(objectql/engine.ts同时被 PR #4972 串行占用)。packages/spec/**及生成物同样零改动。测试:两个方向都钉,且能识破
unref()式假修复沿用 #4950 的写法 —— 断言可观察后果,而不是断言源码里出现了
clearTimeout(后者是任何重构都能满足的同义反复)。每处三条:process.getActiveResourcesInfo()只报告当前真正钉住事件循环的资源,正是 内核的插件 init/start 超时守卫定时器从不清除也不 unref —— 每个进程在工作结束后还要空转 startupTimeout(CLI 挂 ~120s 才退出) #4813 里让os migrate空转 ~120s 的那个属性;vi.getTimerCount()看得见unref()过的定时器,因此这条能区分「守卫被回收」与「守卫只是从事件循环上摘下来了」;unref()会让「无 ref'd 定时器」这条成立的同时把守卫也废掉(进程没别的事做就直接静默退出,超时永不上报),这条钉住反方向。A2 另加一条:同步
destroy()时守卫在同一个 turn 被回收(覆盖上面那处T | PromiseLike< T >放宽)。双向验证过(临时改坏源码跑给自己看,已还原):
hot-reload.test.tsengine.test.ts(Node Timeout)unref()式假修复验证
两个包都没有
typecheckscript(在scripts/check-type-check-coverage.mjs的 DEBT 台账里),因此按台账的口径直接跑各自的tsc --noEmit,并与「把本 PR 改动 stash 掉」的基线对比:即新增 0 条类型错误。
eslint对 4 个改动文件干净。已git merge origin/main(⛔ 未 rebase)并在合并后重装依赖、重建 spec/core、复跑上面两个套件 —— 这一轮main动过service-automation/src/builtin/parse-config.ts与spec/src/automation/*(#4001 批 10 的 strict 化),复跑即为其而做。分支相对main的 delta 仍恰好是 5 个文件。Changeset
.changeset/node-and-shutdown-timeout-guards-cleared.md——@objectstack/service-automation+@objectstack/core双 patch。🤖 Generated with Claude Code
https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX
Generated by Claude Code