Repository navigation
fix(judge): release cleanup locks before restoring failed sessions - #22
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; model=gpt-6.1-sol; provider=OpenAI; source=runtime_reported; reasoning_effort=max
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机 / Motivation
使用 E2B game sessions 的原生 Judge 操作者,以及清理失败后仍要读取状态和继续使用 session 的调用方。
两个 session 清理失败之间并发读取第一个失败 session,旧源形成 global registry lock 与 operation lock 的循环等待;当前 head 先释放所有 operation locks,再恢复失败项。
相同失败顺序下清理能够返回原 aggregate error、session 保持可重试,随后状态读取与成功清理能实际完成;其他 task 和新 session 不被本次快照清理。
本 PR 不修改评分、模型、提交机会、checkpoint 编码、provider 默认 pin 或研究资格,也不授予合并、云资源或实验执行权限。
改动思路 / Approach
失败集合和 session 对象都属于现有 JudgeState 的一次清理;复用原 finally 释放锁,再恢复 registry,避免新增锁 owner、持久队列或重试框架。
本次仅修复 close_all 的失败恢复顺序,并保留原 per-session serialization、aggregate error、成功 archive 与 Docker/K8s 后台清理分支。
原来的 reader 在持有 _game_lock 时等待 session 的 operation lock;close-all 却在仍持有该 lock 时恢复 registry,然后处理另一个失败项并再次请求 _game_lock。这不是 backend 超时问题,延长等待不能打断依赖环。当前改动让失败项在所有锁释放之前保持 detached;reader 可以暂时得到 not-found,但不能通过已恢复的 A 进入锁循环。finally 完成恢复后,原 session ID 可继续读取和重试。
具体改动 / Changes
审查 118f9257d39c71d0805bb5b9cd4eb85d13a1e6d2 对 integration base 2e29d24f44fece91dc240fc5459d17cad19c5fcd 的完整两文件 diff:+100/-5,其中 production 只有9行新增、5行删除。独立依据为变更前 docs/en/reference/judge-api.md,spec revision 2e29d24f44fece91dc240fc5459d17cad19c5fcd:
- Game Mode API / Close All Sessions — implemented:只处理指定 run/task 的当前快照;失败仍返回原 aggregate error,且可在后续成功清理。
- Game Mode API / Get Status — implemented:失败恢复后同一 session 可经原 API 读取;wrong-scope 仍404,清理过程中 detached 状态的临时404保持可见。
关键代码讲解
close_all_game_sessions:1757保留原 snapshot/detach 与 per-session serialization,仅把失败收集改成包含 session 对象的局部 tuple;成功项仍立即 archive,原错误内容和顺序保留。finally:1799先释放本轮所有 operation locks,再在 registry lock 内用原setdefault恢复失败项。这复用了原 owner 和释放边界;没有持久重试队列、兼容协议或新的 actor 权限。game_status:1693自身未改,仍走原 global→operation 顺序;现在不再遇到“已恢复但仍被 close-all 持锁”的失败项。实际 FastAPI status/close-all 路径已在独立夹具中执行,后续 archive 文件也逐项读回。
对主干的风险 / Risks
未发现本精确 head 的阻断问题。最强反例仍是原来的“双 cleanup failure + 两次失败间读取 A”:我以实际导入的 JudgeState、真实 registry/session locks 和不同于作者测试的 event 排序,对同一 base/head 执行。base 两线程锁死,global 与两个 operation locks 保持持有;head 正常返回包含 A/B 的原 RuntimeError,所有锁释放、A/B 恢复,随后 HTTP status200、wrong-scope404、close-all closed2,实际 history 与两个 game_result.json 都只记录一次。
同一独立脚本还覆盖单失败、全部成功、Docker 后台 cleanup 失败,以及另一 task 与清理过程中新增 session 的隔离。base/head 的 ordinary/off 结果相同;head 在失败恢复期间短暂返回 not-found,这是明确的时序变化,不能声称并发 status 总会200,但恢复完成后继续进展已证。把新 concurrency test 原样应用到未改 base 是2passed/1failed;head 为3passed。没有放宽 timeout 或删除原序列化/编号断言。
本地执行了两文件 diff hygiene、Python compile 与 public-boundary 检查。仓库现有 workflow 是文档构建,本次没有查询或等待 GitHub CI。作者报告的 Linux 完整 suite 保留为作者证据,不计为我的独立通过项;未启用 live E2B 或任何模型/评分,因此本结论不宣称云服务与研究收益资格。
我的整体评价 / Verdict
该 bounded fix 对原 F2 反例给出可验证修复,并保留现有成功/失败、scope、错误与后续恢复行为。long_horizon 与 user_experience 的改善在“错误能返回、同一 session 能继续”这一具体路径成立;没有把测试数等同研究效果。代码量与职责划分合理,兼容协议和新公共词汇均不需要;保留现有失败集合与 finally 是本处足够的简化。
已检查两个文件及实际 FastAPI game routes,没有页面、布局或 viewport 变化;验证对象是原生 API 和清理恢复。
本次独立验证为 macOS Python 3.12.15、实际 JudgeState/FastAPI/archive IO 与 synthetic backend;未调用 live E2B、Docker workload、模型或评分,也未独立重跑作者完整 Linux suite。
LoopX 的默认 source 采用和 non-finite checkpoint codec 由 #7828 单独资格验证;这份原生修复批准不自动批准 source 升级、merge 或实验启动。
English verdict: APPROVE for #22118f925. The independent double-failure/status lock cycle is fixed, retry and archive progress are verified, and CI was not consulted.
close_all_game_sessionsholds every detached session's operation lock while cleaning up E2B sessions. If the first cleanup fails, publishing that session before releasing its lock lets a concurrent status read hold_game_lockwhile waiting for that operation lock. A second cleanup failure then waits for_game_lock, producing a lock cycle.Keep failed sessions detached until the
finallyblock has released every owned operation lock, then publish the retryable sessions together. Preserve the existing aggregate error, per-session serialization and successful cleanup/archive behavior.The regression uses two cleanup failures and an event-ordered concurrent status read. It fails against the unchanged shared integration base with both threads blocked. After the fix, the close operation returns its error, the status read returns promptly, all locks are free, and a subsequent successful cleanup archives both sessions once.
Validation on Linux Python 3.12:
This targets the shared
codex/edgebench-integrationbranch. It addresses the provider lock-cycle finding in LoopX #7824; qualified source adoption is tracked separately in LoopX #7828. Non-finite checkpoint encoding remains a separate compatibility requirement.