Skip to content

fix(service-automation,core): 超时守卫在 race 落定时被清除 —— #4813 / #4875 的清仓 (#4952) - #4982

Merged
xuyushun441-sys merged 2 commits into
mainfrom
claude/issue-4952-timeout-guard-cleanup
Aug 3, 2026
Merged

fix(service-automation,core): 超时守卫在 race 落定时被清除 —— #4813 / #4875 的清仓 (#4952)#4982
xuyushun441-sys merged 2 commits into
mainfrom
claude/issue-4952-timeout-guard-cleanup

Conversation

@xuyushun441-sys

Copy link
Copy Markdown
Contributor

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 一样会把一次性进程(os CLI 跑到 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.tsraceShutdownTimeout< T > 把入参放宽到 T | PromiseLike< T > —— Plugin 契约允许同步 destroy(): void(packages/core/src/types.ts),与 kernel / health-monitor 两处放宽的理由相同;
  • engine.tsexecuteWithTimeout 放宽 —— NodeExecutor.execute 声明返回 Promise< NodeExecutionResult >

没有做成跨文件共享 helper。 issue 的派发单允许在 packages/core 内部考虑与 health-monitor.tsraceCheckTimeout 共用。结论是不共用:本轮的硬约束是 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 裁定它们是有意为之的既定语义:

  • B1 packages/core/src/kernel.ts:430 shutdown() —— 显式 unref():进程正在退出,吞掉一次超时是可接受的;
  • B2 packages/objectql/src/engine.ts:2971 checkDriversHealth() —— 显式 unref(),代码里的注释(Never hold the event loop open for a probe)已经把这个选择记录在案。

除非维护者在 issue 上推翻该裁定,这两处保持原样;本 PR 对 kernel.tsobjectql/engine.ts 零改动(objectql/engine.ts 同时被 PR #4972 串行占用)。packages/spec/** 及生成物同样零改动。

测试:两个方向都钉,且能识破 unref() 式假修复

沿用 #4950 的写法 —— 断言可观察后果,而不是断言源码里出现了 clearTimeout(后者是任何重构都能满足的同义反复)。每处三条:

  1. 真实定时器下不留 ref'd 定时器 —— process.getActiveResourcesInfo() 只报告当前真正钉住事件循环的资源,正是 内核的插件 init/start 超时守卫定时器从不清除也不 unref —— 每个进程在工作结束后还要空转 startupTimeout(CLI 挂 ~120s 才退出) #4813 里让 os migrate 空转 ~120s 的那个属性;
  2. fake timers 下连跑多轮不累积 —— vi.getTimerCount() 看得见 unref() 过的定时器,因此这条能区分「守卫被回收」与「守卫只是从事件循环上摘下来了」;
  3. 被守护方真的挂住时,超时照常上报 —— unref() 会让「无 ref'd 定时器」这条成立的同时把守卫也废掉(进程没别的事做就直接静默退出,超时永不上报),这条钉住反方向。

A2 另加一条:同步 destroy() 时守卫在同一个 turn 被回收(覆盖上面那处 T | PromiseLike< T > 放宽)。

双向验证过(临时改坏源码跑给自己看,已还原):

源码状态 hot-reload.test.ts engine.test.ts(Node Timeout)
修复前(armed 后扔掉) 3 failed / 1 passed 2 failed / 3 passed
unref() 式假修复 2 failed / 2 passed 1 failed / 4 passed
本 PR 4 passed 5 passed

验证

pnpm --filter @objectstack/core test              → Test Files 29 passed (29) / Tests 469 passed (469)
pnpm --filter @objectstack/service-automation test → Test Files 55 passed (55) / Tests 665 passed (665)

两个包都没有 typecheck script(在 scripts/check-type-check-coverage.mjs 的 DEBT 台账里),因此按台账的口径直接跑各自的 tsc --noEmit,并与「把本 PR 改动 stash 掉」的基线对比:

packages/core:                          120 errors  ←→ 基线 120(hot-reload.ts / hot-reload.test.ts 一条都没有)
packages/services/service-automation:     5 errors  ←→ 基线 5(全部是既有的 engine.test.ts x2 / nested-region-parity.test.ts x3)

新增 0 条类型错误eslint 对 4 个改动文件干净。已 git merge origin/main(⛔ 未 rebase)并在合并后重装依赖、重建 spec/core、复跑上面两个套件 —— 这一轮 main 动过 service-automation/src/builtin/parse-config.tsspec/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

claude added 2 commits August 3, 2026 18:24
…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
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 3, 2026 6:30pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/core, @objectstack/service-automation.

26 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/ai/actions-as-tools.mdx (via @objectstack/core)
  • content/docs/ai/knowledge-rag.mdx (via @objectstack/core)
  • content/docs/ai/natural-language-queries.mdx (via @objectstack/core)
  • content/docs/automation/flows.mdx (via @objectstack/service-automation)
  • content/docs/automation/webhooks.mdx (via @objectstack/core)
  • content/docs/concepts/north-star.mdx (via packages/core)
  • content/docs/deployment/migration-from-objectql.mdx (via @objectstack/core)
  • content/docs/kernel/contracts/index.mdx (via @objectstack/core)
  • content/docs/kernel/runtime-services/examples.mdx (via @objectstack/core)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/core, @objectstack/service-automation)
  • content/docs/kernel/services.mdx (via @objectstack/core)
  • content/docs/permissions/authentication.mdx (via @objectstack/core)
  • content/docs/permissions/authorization.mdx (via packages/core)
  • content/docs/plugins/anatomy.mdx (via @objectstack/core)
  • content/docs/plugins/development.mdx (via @objectstack/core)
  • content/docs/plugins/index.mdx (via @objectstack/core)
  • content/docs/plugins/packages.mdx (via @objectstack/core, @objectstack/service-automation)
  • content/docs/protocol/kernel/config-resolution.mdx (via @objectstack/core)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/core)
  • content/docs/protocol/kernel/lifecycle.mdx (via @objectstack/core)
  • content/docs/protocol/kernel/plugin-spec.mdx (via @objectstack/core)
  • content/docs/releases/implementation-status.mdx (via @objectstack/core, @objectstack/service-automation)
  • content/docs/releases/v12.mdx (via @objectstack/core)
  • content/docs/releases/v15.mdx (via @objectstack/core)
  • content/docs/releases/v17.mdx (via @objectstack/core)
  • content/docs/releases/v9.mdx (via @objectstack/service-automation)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added the size/m label Aug 3, 2026
@xuyushun441-sys
xuyushun441-sys marked this pull request as ready for review August 3, 2026 18:46
@xuyushun441-sys
xuyushun441-sys added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit c5adfe1 Aug 3, 2026
24 checks passed
@xuyushun441-sys
xuyushun441-sys deleted the claude/issue-4952-timeout-guard-cleanup branch August 3, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

超时守卫「armed 之后扔掉」还剩两处生产实例(另有两处用 unref 代替回收)—— #4813 / #4875 的清仓

2 participants