Skip to content

fix(runtime): require process exit before namespace disposal - #7546

Merged
huangruiteng merged 2 commits into
mainfrom
codex/runtime-process-exit-proof
Oct 29, 2026
Merged

huangruiteng merged 2 commits into
mainfrom
codex/runtime-process-exit-proof

Conversation

@hhyykk

@hhyykk hhyykk commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

When a runtime leader becomes a zombie while another thread can still write, strict namespace shutdown can report stopped too early. This PR removes the process-group shortcut from wait_for_process_exit=True; cleanup must wait for the PID to disappear after real exit/reaping. Ordinary restart keeps its existing serving-locator behavior.

The adjacent child-reaping regression now releases the real child through the shutdown boundary and waits for its exit without reaping it. The runtime owner must then reap it before reporting success. This replaces the obsolete group-observer hook while retaining the PID and waitpid assertions. The Linux surviving-writer regression and owned/inherited process-group cases remain.

Validation at 6550e3ec8fdf4ad4ff5154bf1bb664bc77534009 (based on target main 85a2967cf3de639bf8c14bd930d280dd6103626d):

  • Five complete modules: test_effect_runtime_restart.py, test_runtime_retirement_cleanup.py, test_managed_effect_runtime_fixture.py, test_effect_runtime_compile_cache.py, and test_effect_runtime_integration.py: 151 passed.
  • The original group-observer-dependent case failed before the fixture migration.
  • Ruff, diff hygiene and the RFC bilingual diff check passed.
  • Quick premerge passed: 4 direct checks, changed-Python compile, 2 selected catalog canaries, and the public/private boundary scan; no failures or manual holds.

Limits: the zombie-thread regression is Linux-specific; a present/unreaped PID remains shutdown_pending. Local validation does not establish Windows, installed App, or long-running production health. Merge remains a maintainer decision.

中文:严格清理须等待真实退出并完成必要回收;修复旧测试对已删除观察钩子的依赖,保留实际子进程、PID 和 waitpid 覆盖。普通重启行为不变。本次只修命名空间清理边界。

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent — gpt-6.1-sol / OpenAI; runtime_reported; reasoning_effort=max

REQUEST_CHANGES — 一个 P2:迁移被删除观察钩子的既有回收测试。 运行时修复方向有实证;当前相邻完整测试集仍出现本次删除直接引入的失败。

动机 / Motivation

维护者运行 smoke 或测试,随后清理其自有临时目录时会遇到这个问题。 例如主线程已退出、工作线程仍等候写文件时,旧实现先报告已停止;新实现应保留目录,等整个进程真正退出后再允许清理。 误报已停止会让清理与退出期写入交错,留下被重建的临时目录或丢失失败现场。

独立 Linux 实进程对照确认旧实现提前成功、新实现保持 pending,释放工作线程并回收后才返回 stopped。 运行时安全增量已得到实证,但一个依赖被删除观察钩子的既有测试在当前 head 失败,仍须修正其夹具并重验。 这次不交付新的 controller 会话、正式安装、持续运行稳定性或长程成本收益。

改动思路 / Approach

这个边界应复用原 _stop_effect_runtime,把“服务已交接”和“可以删除目录”分开。runtime.shutdown 的认证请求只是停机意图;wait_for_process_exit=True 必须继续等 PID 消失或对子进程完成真实回收。当前改动删除进程组快照捷径,没有新增状态、配置、provider 或审批轮次。保留这段删除比再维护一套按 ps 文本分类的推断更合适;普通 restart 的非严格分支原样保留。

原有停止 owner 通过删除错误捷径实现私有目录安全,普通交接另有现成契约;不增加状态或等待审批。 私有 namespace 删除前的真实停止不变量;修复相邻旧夹具后再批准,不扩 controller。Linux 上真实工作线程在停机返回后仍能写 witness 的对照说明它有独立价值,无须把完整双会话/controller 资格变成前置。下表是实际同输入观察;TCP peer 是合成的认证响应端,PID、线程、写文件和 waitpid 都是真实 OS 行为。

条件 不可变 base 当前 head
主线程 Z、工作线程仍活跃,独立进程组 stopped,随后仍实际写文件 shutdown_pending,退出并回收后 stopped
相同情况、继承进程组 stopped,随后仍实际写文件 shutdown_pending,退出并回收后 stopped

具体改动 / Changes

全文 base 23bb8c89d6a01794faf75330170cfc19402bb6ab 到 head 4a8b5058d229b4e766ef761813a5f9259d2f3207 共三条路径:production +1/-16,测试 +47/-2,双语开发文档 +12。没有机械搬移、生成文件或第二个决策 owner。

规范依据为 docs/development/testing-and-quality.md,spec_revision 23bb8c89d6a01794faf75330170cfc19402bb6ab。这里读的是本次修改之前已存在的 cleanup 条款;以下 NS 标签仅用于映射该段无编号要求:NS-EXIT implemented,真实退出前不允许清理;NS-RETAIN implemented,runner 失败保留目录和原输出且不跟随链接;NS-OPERATOR implemented,普通 restart 仍按服务交接返回。新增文档解释与此原要求一致,不能拿新增段落自证。

关键代码讲解

  1. restart_effect_runtime 默认 wait_for_exit=False,显式严格调用才进入物理退出判断,未把全部普通重启升级为破坏性清理门。
  2. _stop_effect_runtime 先发原认证 shutdown,再在严格分支执行 reap/PID 检查。删除 ps 捷径后,仍存在的 PID 保持 pending,locator 换人也不能让此分支提前成功。
  3. _reap_exited_runtime_child 沿用非阻塞 waitpid。非本进程子节点不能冒领其回收责任;父进程真正回收后可重试,不靠“Z 就不会写”的文本假设。
  4. _retire_fixture_runtimes 只读取自有新建 namespace 的合法 locator,严格结果未 stopped 即抛错,_remove_fixture_root 因而不会继续删除。实际集成测试还覆盖 locator 替换/消失与有无独立组的组合。

对主干的风险 / Risks

[P2] 保留退出/回收语义,改掉依赖已删除钩子的夹具

当前 tests/control_plane/test_effect_runtime_restart.py::test_namespace_shutdown_reaps_child_exiting_during_group_observation 让 child 一直等 stdin,并且只在 monkeypatch 的 _posix_owned_group_has_exited 回调里关闭 stdin。删除该 helper 调用后,回调永不触发、child 从未被要求退出,最终得到 pending,stopped 断言失败。这是测试编排与本次删除不兼容,不能据此推断运行时仍会误删活跃 writer,也不能忽略它对当前 pytest 的破坏。

独立执行四个相邻完整模块得到 50 PASS / 1 FAIL;将唯一变化的 runtime owner 换成不可变 base,同一个未修改测试 1 PASS。该对照是 immutable owner 模块与相同测试/相邻依赖,未冒称完整 base 安装。最小修复是在本 PR 中让真实 child 的退出由独立同步事件触发,保留“退出后 reap、PID 消失、重复 waitpid 得 ChildProcessError”的实义断言,并重跑 uv run --extra test python -m pytest -q tests/control_plane/test_effect_runtime_restart.py tests/control_plane/test_runtime_retirement_cleanup.py tests/test_managed_effect_runtime_fixture.py tests/control_plane/test_effect_runtime_compile_cache.py;不要恢复已经证伪的 ps 捷径,也不要删掉有效回收覆盖。

语义与 CI 对齐

共享状态仍复用现有 stopped / shutdown_pending,没有新 vocabulary 或 prose 分类器。严格清理的失败是机器执行结果,文档没有把它说成可忽略建议;普通服务重启与严格目录删除的默认差异已经明确。按当前审查配置只做本地验证,未查询、等待或推断远端 CI。

当前 head 的完整 integration 模块 99 PASS / 1 Linux 平台 SKIP;独立 Linux arm64/Python 3.11 实进程对照补足上述线程场景,但没有称原 pytest Linux case 已执行:独立镜像没有 pytest。macOS/Python 3.14.8 的相邻完整集保留上述失败;Ruff 通过,standard premerge 6 direct + 11 selected PASS、0 hold。首次私有 Linux 探针对“继承组 base 应 pending”的预期错误已修正并保留;它是核验器问题,不是作者缺陷。测试通过与物理退出、真实 Node 缓存及长期稳定性仍分开。

我的整体评价 / Verdict

这是有正向价值、归属正确、体量适当的内部安全增量;long_horizon 与 user_experience 在所测停止/重试路径保持,未认证长期收益。没有新增 opt-in 能力,wait_for_exit=False 的默认路径已对照保持;公开协议没有扩大 actor 或 authority。当前 PR 边界保留为命名空间清理修复,不扩成 controller 重构。

仍需 REQUEST_CHANGES:修正现有测试对旧实现钩子的依赖并让完整相邻回收覆盖通过后再审。视觉检查不适用:所查为 runtime/runner/fixture,没有渲染改动;正式 App、Windows、真实 Node 退出缓存和长期效用仍未验证。没有合并、安装或 dismiss 权限推断。

English verdict: REQUEST_CHANGES at 4a8b5058d229b4e766ef761813a5f9259d2f3207. The process-exit safety fix is reproduced, but removing the group observer breaks an existing reaping test; preserve its semantic coverage and repair its trigger before approval.

# before returning successful namespace cleanup.
_reap_exited_runtime_child(info)
break
# A zombie leader can still have threads writing into the namespace.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 删除该调用后,现有真实回收测试的 child 永远等 stdin;唯一关闭 stdin 的 monkeypatch 回调再也不执行。当前四模块50过1败,替换不可变 base owner 的同一测试过。请迁移退出触发、保留真实 waitpid/PID 断言;不要恢复已证伪的 ps 快捷成功或删除有效回收覆盖。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed at 6550e3ec8fdf4ad4ff5154bf1bb664bc77534009. The real child is released by the shutdown request and observed with waitid(WNOWAIT), while the runtime owner must reap it before reporting stopped. The PID and waitpid/ChildProcessError assertions remain; the deleted process-group observer is not restored. All five adjacent complete modules passed (151 tests); Ruff, RFC pairing and quick premerge passed. The PR is based on refreshed main 85a2967cf3de639bf8c14bd930d280dd6103626d.

Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com>
(cherry picked from commit f5ace94af3690a002978628b02f56cdd59727adc)
Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com>
(cherry picked from commit 4a8b505)
Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com>
Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com>
@hhyykk
hhyykk force-pushed the codex/runtime-process-exit-proof branch from 4a8b505 to 6550e3e Compare October 29, 2026 03:52

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · reasoning_effort=max

APPROVE — 原退出安全修复成立,旧回收夹具的阻断也已修复。 本次完整审查 head 6550e3ec8fdf4ad4ff5154bf1bb664bc77534009,基点 652f503842609e948e8ff832ae3977f23f79f28d;另行比较上次 4a8b5058d229b4e766ef761813a5f9259d2f3207,没有继承旧结论。

动机 / Motivation

维护者清理 smoke 自有临时目录时,需要确认运行时已经真正退出。 主线程已成僵尸、工作线程仍能写文件时,旧实现提前报告 stopped;当前实现保留目录并返回 shutdown_pending,退出回收后再成功。 误报成功会让删除目录与退出期写入交错,丢失失败现场或重新生成目录。独立实进程对照和现有完整模块证明严格清理等待真实退出,原回收测试也已恢复通过。 本 PR 只收口现有 namespace 停止边界和其回归夹具,不增加配置、状态或新的审批轮次。

改动思路 / Approach

原停止 owner 的 OS 观察应承担目录销毁不变量,删除不可靠捷径比添加状态或第二套进程分类更简单。 本 PR 只收口现有 namespace 停止边界和其回归夹具,不增加配置、状态或新的审批轮次。 原 authenticated shutdown 负责请求排空,原 Python OS adapter 负责观察退出;同一 PID/token 和原返回 schema 保持。普通 restart 继续使用服务 locator 交接,严格目录清理才等进程真正退出。非 child 僵尸无法由此调用者回收时继续 pending 是准确限制,不能用假成功换取及时清理。

本次没有新模块、状态表、配置或并行生命周期 owner。相邻测试把真实 child 的退出触发移到 shutdown 响应边界,仍用 waitid(WNOWAIT) 只观察,再要求原 owner 实际 reap。不会恢复已被反例证伪的 ps shortcut,也没有删除 PID 消失和重复 waitpid 断言。

具体改动 / Changes

完整四路径 +68/-38:生产 +1/-16,两个测试 +55/-22,文档 +12。修改前的接受依据是 docs/development/testing-and-quality.md,spec_revision 652f503842609e948e8ff832ae3977f23f79f28d。以下 NS 标签映射原段落的无编号条款:NS-EXIT implemented,实际退出前不销毁;NS-RETAIN implemented,失败保留现场和原输出、不跟随链接;NS-OPERATOR implemented,普通 restart 保留服务交接语义。

关键代码讲解

  1. restart_effect_runtime 默认 wait_for_exit=False,未将普通重启变成目录删除门。
  2. _stop_effect_runtime 严格分支先 reap、再检查 PID;删除组状态快捷判断后,仍存在的进程保持 shutdown_pending。
  3. _reap_exited_runtime_child 沿用非阻塞 waitpid,不冒领非 child 的回收责任。canary/runner.py:283 消费同一停止结果,未成功即保留 namespace。

独立 Linux 实进程对照使用相同 fixture、原生产客户端和认证 TCP 响应端。响应端是合成的,进程、线程、waitpid 与文件写入是真实 OS 行为:

组长已Z、writer仍活跃 不可变基点 当前head
独立进程组 提前stopped,返回后仍可写 pending,退出回收后stopped
继承进程组 提前stopped,返回后仍可写 pending,退出回收后stopped

对主干的风险 / Risks

没有新的阻断发现。上次请求修改针对“唯一关闭 stdin 的旧 helper 回调不再执行”;本头改为 shutdown 关闭真实 child 输入,保留 waitid不回收、owner必须reap、PID消失及再次waitpid抛错的全部语义。这次五个完整模块独立得到 150 PASS / 1 Linux平台SKIP,旧失败用例已通过。Linux容器另执行上述两组生产边界对照,覆盖macOS不能运行的活跃线程场景;它不等于原Linux pytest节点或真实Node服务已在容器执行。

标准 premerge 4 direct、changed-Python compile、2 catalog、8 risk、1 public-boundary全部PASS,0 manual hold;变更Python Ruff、双语差分检查和两个提交DCO通过。原compile-cache、locator turnover、restart、cleanup和实际runtime集成覆盖均保留。没有用绿测抵消尚存缺口,也没有查询、等待或引用CI作为独立验证。

语义与 CI 对齐

复用既有 stopped / shutdown_pending vocabulary,删除不可靠的OS组状态推断,没有新增actor或权限模型。strict失败是机器执行结果,说明未称其为可忽略建议。普通 wait_for_exit=False 的默认、输出schema和原鉴权/namespace归属保持;这是已有严格清理不变量的修复,没有新default-off能力。

我的整体评价 / Verdict

这是完整且可回滚的内部安全修复,机制归属和体量适当。long_horizon preserved:原身份、失败现场与退出后的重试保持;user_experience improved:清理不会在活跃writer下假成功,无额外人工准备或确认。已有owner足够,当前删除已落实有界简化,无需另建跟踪任务或兼容shim。

所查 effect_runtime、canary runner 和测试没有渲染界面改动,视觉检查不适用。 视觉及平台未验证范围:正式安装、原生 Windows 和持续运行健康未在本轮资格化;Linux 独立探针使用容器和合成认证 TCP peer,不是真实 Node 服务。 这些限制不被本地通过改写为安装或长期收益资格。批准仅针对当前精确head的这项修复;合并仍取决于当前维护者授权及原生readiness。

English verdict: APPROVE at 6550e3ec8fdf4ad4ff5154bf1bb664bc77534009. The whole four-path change fixes premature namespace disposal, preserves real child reaping, and passes150 tests with one platform skip plus independent Linux base/head counterfactuals. Standard local premerge passes; no merge or installed-runtime qualification is claimed.

@huangruiteng
huangruiteng merged commit c4efa10 into main Oct 29, 2026
31 of 41 checks passed
@huangruiteng
huangruiteng deleted the codex/runtime-process-exit-proof branch October 29, 2026 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants