Repository navigation
Fix file mutation lock cleanup - #8409
Conversation
Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · max
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机 / Motivation
用户在 LoopX 打开「委派 → 全部记录」时,服务会逐批读取历史并释放用于保护这些读取的锁。
每次释放会建立一个临时声明,以保证同一时刻只有一个释放者。原来,释放者清理自己创建的声明时会重读文件;一旦声明内容损坏成无效 UTF-8,清理会中断并留下文件。改动后,释放者按创建时捕获的文件身份清理自己的声明;如果同一路径已换成另一个文件,则保留它。
在同源的八对隔离 HTTP 读回里,所有者记录读取每次从 1,696 次减为 848 次;历史结果一致,撤销权限和非法分页仍被拒绝,恢复后也能读回同一结果。
此 PR 交付声明清理和减少重复 I/O,未改变委派权限、历史排序和分页,也未改变页面呈现。 八对里五对总耗时缩短,但两组各自的中位耗时没有下降,仍需在正式入口重复验证长尾和端到端耗时。
改动思路 / Approach
锁的排他、所有权复核和跨运行时持久化仍是必要约束;用现有文件身份清理本进程声明,能移除额外解析且不用另设一套锁。 本次只改 Python 释放适配和回归测试;剩余耗时由原委派读取路径负责,正式验收须同时比较同源结果、拒绝和恢复。
核对 Python 旧实现,发现声明内容正常和内容无法识别的分支最终都调用相同的文件身份校验;读 JSON 没有决定是否删除。只捕获解码异常可以补上一个坏文件场景,却不会去掉普通路径的冗余 I/O。现有原生 TypeScript 释放端继续使用相同的磁盘契约,不需要一起改。
具体改动 / Changes
核对的提交是 817d6ec053106936169124087c665c8dbaf35212,基线是 71190b6b4ef4e146f3bf4a10d31ce61ac42fd021。远端 diff 与本地审查 diff 的稳定 patch ID 一致。
关键代码讲解
_release_effect_mutation_claim:拿到本进程独占创建时的文件身份,便直接复用受保护的删除函数;只有拿路径找回声明时,才读进程 ID 与 token。即使替换文件复制了相同的 token 和内容,其文件身份也不同,不能删除。_release_effect_mutation_lock:拿到独占声明后仍重新检查原标记的 token 和身份,匹配才改名退出活跃路径。第一次删除已退役标记成功才跳过重复删除;如果遇到短暂 I/O 失败,原回退仍会再试一次。清理临时声明的外层重试也保留。test_cross_runtime_claim_cleanup_removes_own_corrupted_claim:原先只测可解码但非法的 JSON。补上不可解码字节后基线确实失败,当前提交通过。同文件另有同内容替换和两类临时删除失败测试;去掉外层声明重试的中间版本会留下声明并触发失败。
对主干的风险 / Risks
最需防的是清理时误删接任者的声明,或删除失败后留下一把实际上已完成的锁。现有 token 与身份复核、独占声明和原子改名均保留。测试覆盖同内容替换、坏字节、活跃/过期持有者、错误 token,以及两种临时文件删除各失败一次。持续 I/O 故障及身份检查后遭外部程序改写仍由既有恢复边界处理,此改动不扩张锁模型。
本地锁测试 30 项通过、2 项平台跳过;File/SQLite 原委派 HTTP 测试 2 项通过,静态检查和本地预合并通过。全仓补充测试提前停止,不能当作全量通过。隔离配对只支持减少读取次数;生产长尾未验证。此评审按当前规则使用本地验证,未查看远端 CI。
我的整体评价 / Verdict
改动和证据足以支持这个范围内的清理修复:坏文件可恢复,替换文件与原重试仍受保护,原委派读回未变。后续再单独验证正式入口是否提速。此为作者复核,独立评审与合并资格仍待确认。
English verdict: APPROVE this bounded lock cleanup at the reviewed commit; production latency and independent approval remain open.
Signed-off-by: huangruiteng <huangrt01@163.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Reviewer: model_agent, GPT-5, OpenAI, self_reported
Independent approval
动机 / Motivation
通过委派历史等 HTTP/CLI 读取路径进入跨运行时 mutation lock 的本地进程,以及依赖这些读取正常清理锁文件的运维者。旧路径在释放自己创建的 claim 时重新读取并解析 claim JSON;如果 claim 被损坏成 invalid UTF-8,解析异常会中断 cleanup,留下锁协调文件。普通 cleanup 还多了一次不需要的 owner 读取。base 上创建 claim 后写入 b'\xff' 并释放,release 抛 UnicodeDecodeError 且 claim 仍存在;head 上同一场景 claim 被删除。若路径被替换为同内容的新 inode,head 保持 replacement 不动。
head 的锁和 cross-process 测试 33 项通过、2 项平台跳过;delegation/lock 路径 69 项通过;base/head 手工对照确认坏 claim 从残留变为清理,且 replacement inode 未被误删。
本 PR 不证明生产 HTTP 长尾 latency 或所有 Windows 文件系统的 identity 行为;这些需要正式环境的补充测量。
改动思路 / Approach
入口仍是 Python 的 cross-runtime mutation lock release,权威状态是 kernel lock、token-bound claim path、创建时捕获的文件 identity、owner record 和 retired marker。_release_effect_mutation_claim 在 owned-claim 分支只使用创建时捕获的 inode identity,因此即使内容不可解析也能清理本进程文件;pathname/token recovery 仍读取 owner 并核对 PID/token。资格是无持久状态的可复用投影之外的内部 cleanup 规则:本进程独占创建 claim 才获得 owned branch,文件 identity 不匹配就保留 replacement。
使用已捕获的 inode identity 清理本进程 claim,保留 pathname/token 分支和 marker retry,是最小且与现有锁模型一致的变更。本次只修复本地 lock cleanup 与回归覆盖,不宣称生产 latency 或跨文件系统完成度。_release_effect_mutation_lock 保留 owner token、lock identity、retired marker 和 claim 的 best-effort retry;没有新增锁类型、持久状态、CLI 或权限模型。更小的只捕获 UnicodeDecodeError 方案会让普通路径继续做无意义 I/O,也没有使用已经存在的 identity,因此不采用。
| 场景 | 旧行为 | 当前 head |
|---|---|---|
| 本进程 claim 内容为 invalid UTF-8 | 解析抛错,claim 残留 | 按 captured identity 删除自己的 claim |
| 同一路径替换成同内容新 inode | 不能删除 replacement | identity 不同,replacement 保留 |
| 替换 token 或 owner record | 不能删除其他 owner 的 lock | pathname/token 分支继续核对 owner |
| claim 或 retired marker 首次 unlink 瞬态失败 | 可能留下 marker | 内层和外层 cleanup 各提供一次安全重试 |
具体改动 / Changes
本次核对 e7e9857ac7ba2abb7ccbd6510b096413828a3a4d 相对 9837572bdab00ead43940a23e219d9b32cd6ef3f 的 2 个文件,+64/−38。作者声明没有新的 lock protocol;现有 file-lock acquisition 的 kernel lock 与 holder recovery 是相关语义,但临时 claim cleanup 本身没有独立书面规范,因此本 review 使用已复现的 cleanup defect 和既有 identity invariant 作为验收基准。
关键代码讲解
_claim_effect_mutation_lock在发布 claim 内容前从未关闭的 descriptor 捕获(dev, ino, birthtime, ctime);这条 identity 是后续只删除本进程文件的依据,写入/fsync 失败仍会在异常路径清理。_release_effect_mutation_claim对_EffectMutationClaim不再解析 JSON,而是直接让_remove_created_effect_file比较 identity;传入普通Path时才读取 owner 并要求 token 匹配。_release_effect_mutation_lock保留原 lock-file owner/token/identity 三重检查,并将 retired marker 的首次清理结果记录为retired_cleaned;失败时在 finally 中再尝试一次,claim 也在外层 finally 做最后一轮 best-effort 清理。
测试覆盖本进程坏 claim、替换 inode、替换 token 以及 claim/retired marker 的一次性 unlink 故障。它没有把多次持续 I/O 故障伪装成成功,也没有扩张 lock recovery API。
对主干的风险 / Risks
没有新的 blocking finding。最强反例是 replacement claimant 在旧 claim 路径复用同一个 token 文件,或者 retired marker 的第一次 unlink 失败后留下锁 artifact;当前实现的 owned branch 只有 captured identity 匹配才删除,pathname/token branch 只有 owner PID 和 token 同时匹配才删除,marker 失败则保留安全重试。测试直接验证 replacement inode 内容不变和一次瞬态故障被吸收;持续 I/O 故障仍由既有 OSError/suppress_errors 边界处理。
exact head 验证:tests/test_file_lock.py 与 cross-process lock 用例 33 passed, 2 skipped;delegation/lock 路径 69 passed;Ruff 和 git diff --check 通过。base/head invalid-UTF8 replay 独立确认 base 抛 UnicodeDecodeError 且 claim 存在,head 删除 own claim、保留 replacement。远端 CI 的 pending/failing 项作为独立 merge-readiness 观察保留;视觉审查不适用,因为这是本地文件锁和 HTTP read adapter,没有渲染页面。
语义与 CI 对齐
本 PR 是 local-only 实现变更,candidate decision 为 local_only;它复用现有 lock vocabulary,没有新增公开 schema、命令或 authority。CI 未决定本 review verdict,changed lock invariant 由 exact-head local suite 和 real delegation path 独立覆盖;与 CI 状态相关的内容继续作为 merge readiness 记录,而不是被声称为通过。
我的整体评价 / Verdict
长程效果是改善的:释放路径少了重复 owner 读取,损坏 claim 不再把后续委派读取拖进不可恢复的 cleanup 状态;用户路径和权限拒绝保持原样。实现复用了已存在的 identity、claim path、owner check 和 retry,没有发现重复 lock authority 或未披露的默认变化。残余风险是生产 latency、安装后行为和平台差异仍未测量,不阻塞这个 bounded cleanup fix。
English verdict: APPROVE for e7e9857. The owned claim now cleans safely from captured file identity even with invalid UTF-8, while replacement inodes, token mismatches, and transient marker cleanup failures remain protected.
Goal And Delivered Outcome
Delegation history repeatedly releases shared file locks. The Python release path re-read claims whose file identity it already held, and a claim containing invalid UTF-8 could interrupt cleanup and leave the file behind.
Cleanup now uses the captured identity for an owned claim. Cleanup by pathname still checks the PID and token. Marker release still checks the token and file identity before retirement, and transient failures removing a claim or a retired marker still get a retry.
Intended base:
main.Author Declaration
model_agent, OpenAI GPT-6.loopx/file_lock.pyandloopx/control_plane/effect_runtime_io.tsat71190b6b4ef4e146f3bf4a10d31ce61ac42fd021. There is no new specification._release_effect_mutation_claim_release_effect_mutation_lock_release_effect_mutation_lockScope And Continuation
The patch changes Python file cleanup and its regression tests. It removes half the owner-record reads in a matched inventory read. End-to-end timings varied, so production latency remains a separate verification step. No new lock protocol, storage format, setting, or user action is needed.
Validation
817d6ec053106936169124087c665c8dbaf35212.finishedfor the focused checks.synthetic,authorized_private_read_only.staticpassedunitpassedtests/test_file_lock.py: 30 passed; 2 platform skips on macOS.real_entrypointpassedregression_paritypassedstaticpassedunitblockedThe results apply to the proposed commit and the isolated HTTP read. They do not establish production latency or installed behavior.
Frontend / Visual Evidence
none.Type of Change
LoopX Area
Boundary Checklist
none.