修复 AGC 项目写锁残留无法回收与启动失败无诊断 #313
Reference in New Issue
Block a user
Delete Branch "fix/agc-stale-lock"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
背景
Fixes #310
异常退出后残留 lock 文件导致下次打开无法正常运行。定位到两个独立问题面:项目级
.agent/project.lock残留无法回收,以及启动失败时客户端零诊断。修复前复现
1. 项目写锁残留(Rust 单测)
create_new与落盘之间留下的 0 字节锁:mtime 回拨 120 秒后仍报项目正在被其他写操作占用:<项目>\.agent\project.lock,原实现要等满 600 秒才回收。2. 启动期 GUI owner 锁被活进程持有(真实二进制,隔离
--config-dir双实例)diagnostics/startup.log只有startup.appdata.configure.complete,没有失败行,也没有任何弹窗。阴性结果
agent-runner.gui-owner.lock/agent-runner.lock确实残留,但下一次启动完全正常:OS 独占句柄随进程退出释放,残留文件本身不阻塞启动。落地方案
项目写锁回收(
project/filesystem.rs)processStartedAt,身份不一致即回收createdAt加 5 秒容差即回收启动诊断(
main.rs)StartupLogSlot:配置目录就绪前按应用标识符推导 APPDATA 路径,就绪后切换到真实配置目录;startup.*.failed与show_startup_error_dialog不再是死分支。验证
project_lock_recovery6 条回归用例,含"身份一致不抢锁""新鲜空锁不抢锁"两条护栏。cargo test --bin genarrative-ai-game-creator-shell project_write_lock:11 passed / 0 failed。cargo test --bin genarrative-ai-game-creator-shell diagnostic_log:7 passed / 0 failed。cargo test --bin genarrative-ai-game-creator-shell lock:120 passed / 2 failed / 2 ignored;两条失败(background_agent_runtime_bounds_duplicate_agent_message_livelock、background_agent_runtime_starts_oldest_pending_task_after_lock_acquisition)在干净origin/master上同样以Timeout失败,与本次改动无关。startup.runner.owner-lock.failed details=AI 游戏创作界面已由同一 AppData 目录中的其他进程运行,并弹出可见提示。cargo fmt --check、npm run check:encoding(4333 文件)、git diff --check通过。未覆盖
接手跟进:Native shell tests 失败已修复(
b7f27721c)CI 失败定位
run 1901 只有
Native shell tests失败,唯一失败用例是本次新增的tests::project_lock_recovery::project_write_lock_reclaims_dead_owner_pid(Linux):根因是 fixture 的跨平台假设:
DEAD_OWNER_PID = 0xFFFF_FFF0在 Windows 上OpenProcess返回ERROR_INVALID_PARAMETER(判为已退出),但 Unix 的pid_t是有符号 32 位,i32::try_from(0xFFFF_FFF0)直接失败,project_write_lock_process_is_alive走?返回None(无法判定),于是落回 600 秒保守分支。这条用例在 Linux 上实际没有测到“死进程”分支。本次改动
project_write_lock_process_is_alive把“平台不可能分配出的进程号”(0 或超出平台 pid 宽度)判为Some(false);这类 PID 只可能来自被改写 / 截断损坏的锁文件,直接回收,不再等 600 秒。DEAD_OWNER_PID改为i32::MAX as u64 - 1,同时落在两个平台进程号空间之外且在 Unix 有符号 32 位范围内。project_write_lock_reclaims_unrepresentable_owner_pid(pid = u64::MAX)覆盖上述新判据。decision-log的 PID 边界决策与pitfalls的跨平台 fixture 经验。验证
b7f27721c):4 个 job 全绿,Native shell tests成功;bin 测试2344 passed; 0 failed; 15 ignored,project_lock_recovery7 条在 Linux 上全部通过。cargo test --bin genarrative-ai-game-creator-shell project_lock_recovery→ 7 passed / 0 failed。..._unrepresentable_owner_pid失败,说明新用例确实覆盖该分支。cargo fmt --check、npm run check:encoding(4329 文件)、git diff --check通过。分支已基于最新 master(
04128eb66),无落后提交,mergeable=True。Summary
The lock-recovery changes correctly address the #310 stuck-lock cases: unrepresentable PIDs are treated as dead, empty/corrupt bodies get a 30s grace instead of 600s, and live PIDs are cross-checked with
processStartedAt(with a createdAt+5s fallback for old payloads). Startup diagnostics are a real fix on Windows —StartupLogSlotplusMessageBoxWmakesstartup.*.failedreachable, and the APPDATA+identifier fallback matches Tauriapp_config_dir. The dominant remaining risk is that reclaim is still check-then-remove_filewith no content/nonce CAS, so the new, more aggressive reclaim paths can delete a lock that a concurrent acquirer just installed.Issue counts by severity
@@ -2331,0 +2410,4 @@// 配置目录确定之前先按标识符推导 APPDATA 下的日志路径,确定后再切到真实配置// 目录,保证 `configure_game_creator_runtime_config_dir` 自身失败也有落点。let startup_log = Arc::new(StartupLogSlot::new(std::env::var_os("APPDATA")[suggestion] The early slot is only
APPDATA/<identifier>/diagnostics/startup.log. That matches Windows Tauriapp_config_dir(%APPDATA%\world.genarrative.ai-game-creator), which is why the dual-instance owner-lock dialog works there. It is still a dead diagnostic path whenAPPDATAis unset:StartupLogSlot::failno-ops without a path, andconfigure_game_creator_runtime_config_dir’sinspect_erris itself gated onsetup_log.path(). Non-Windows and stripped-env Windows therefore still get no log and no dialog for the one failure this fallback was added to catch.--config-diris also applied before the slot is created, but the initial path ignores that already-known directory, so a configure failure under a custom config dir is recorded under APPDATA (if any) rather than the dir the process is actually using.Suggestion: Seed the slot from
game_creator_runtime_config_dir()when it is already set, otherwise from the platform config root (APPDATA on Windows,dirs/XDG_CONFIG_HOME/~/Library/Application Supportelsewhere).fail()should still surface a dialog even if the path is unknown (message without a file), instead of going silent.@@ -129,2 +238,3 @@.map(|elapsed| elapsed.as_secs()).and_then(|modified| modified.duration_since(UNIX_EPOCH).ok()).map(|duration| duration.as_secs()).unwrap_or_default()[suggestion] Age and the old-lock PID-reuse heuristic now treat mtime as an absolute Unix timestamp, but
project_write_lock_file_modified_secondsmaps a missing/pre-epoch mtime to0. The previousmodified().elapsed()fallback was0seconds of age (do not reclaim). The new default is Unix epoch, sounix_timestamp() - 0is huge: an unreadable mtime on an empty/corrupt body reclaims immediately, and an old-format live lock with pid but nocreatedAttakeslock_created_at = 0and thenactual > 5, which looks like PID reuse and steals a live holder. Normalcreate_newcrash leftovers have a real mtime, so this is a fallback inversion rather than the main #310 path.Suggestion: Keep a distinct “mtime unknown” value. Unknown age should stay 0 (do not reclaim); unknown
lock_created_atshould not satisfyactual > lock_created_at + 5.@@ -149,2 +262,2 @@if let Some(owner_alive) = owner_pid.and_then(project_write_lock_process_is_alive) {return !owner_alive;let created_at = project_write_lock_owner_created_at(path);let Some(owner_pid) = project_write_lock_owner_pid(path) else {[bug]
project_write_lock_can_be_reclaimedis only a snapshot observation, butacquire_project_write_lockstill unlinks the path unconditionally after this returns true (andremove_fileNotFoundis treated as a hard failure). The lock file is a closed marker, not an OS hold, so the window is real: process B decides a stale/empty lock is reclaimable, process C deletes it and installs a live payload, then B'sremove_filedeletes C's live lock. Two instances recovering the same crashed project — the actual #310 restart shape — now hit this path after 30s of empty-lock grace (and on PID-reuse reclaim), whereas Drop already refuses to delete on content mismatch. The helper also re-reads the file separately forcreatedAt,pid, andprocessStartedAt, so a replacement between those reads can mix a dead pid with a new inode and still return true.Suggestion: Parse the payload once, and only unlink if the bytes/nonce still match that snapshot; treat
NotFoundas “already gone” and retrycreate_newinstead of returning清理失效项目写锁失败. The same-process Drop path already has the content-match pattern to copy.@@ -151,0 +274,4 @@// 新锁自带启动身份:同一进程的身份恒定,不一致即为 PID 复用。(Some(stored), Some(actual)) => stored != actual,// 旧锁没有启动身份,只能用“启动时间晚于锁创建时间”推断 PID 复用。(None, Some(actual)) => {[suggestion] The claimed contract is “live holder is never stolen” and “unknown-alive still waits 600s”, but the new tests do not cover those on the old-payload path.
project_write_lock_keeps_matching_process_identityonly feeds a matchingprocessStartedAt. The createdAt+5s heuristic at this line is the one that can false-positive (clock rollback, or the mtime-epoch fallback in Issue 2), and nothing asserts that a live process whose start time is beforecreatedAtkeeps the lock. There is also no case whereprocess_is_alivereturnsNoneand a fresh lock must survive until 600s.Suggestion: Add an old-format fixture (pid + createdAt, no
processStartedAt) whose live child started beforecreatedAt, expecting contention; and an unknown-liveness fixture that is younger than 600s, also expecting contention.@@ -0,0 +105,4 @@}/// 复现 A:崩溃发生在 create_new 成功、payload 写盘之前,留下 0 字节锁。/// 现在要等满 600 秒才会回收,重启后 10 分钟内所有写操作都失败。[suggestion] Several new comments narrate the pre-fix bug as current behavior, or over-claim the code. This test still says “现在要等满 600 秒才会回收”; the PID-reuse test says “只要那个进程还活着,残留锁就永远不会被回收”. After merge those sentences are false, so the next reader will think the tests document live product rules. In
filesystem.rs:264the empty-body branch says missing pid “只可能是” a crash betweencreate_newand payload write; that branch also covers unreadable files, non-JSON, and JSON without a numericpid.Suggestion: State the invariant the test now enforces (“empty body older than the 30s grace is reclaimed”), and drop the “只可能是” claim — missing pid is “no usable holder”, not a proven crash.
@@ -1333,0 +1335,4 @@- `.agent/project.lock` 新增 `processStartedAt`(持有进程启动时间,Unix 秒)。PID 仍存活时必须先核对启动身份:身份不一致即判定 PID 复用,可直接回收;旧锁没有该字段时退回“进程启动时间晚于锁 `createdAt` 加 5 秒容差”的推断。崩溃停在 `create_new` 与落盘 payload 之间的空锁 / 坏锁宽限期从 600 秒收紧到 30 秒;无法判定持有者是否存活时继续按 600 秒保守回收,活持有者仍然不回收。- 启动诊断日志改为 `StartupLogSlot`:`configure_game_creator_runtime_config_dir` 之前按应用标识符推导 APPDATA 路径,成功后再切换到真实配置目录,`startup.*.failed` 与 `show_startup_error_dialog` 不再是死分支。Windows 启动失败恢复系统消息框并附诊断日志路径,其它平台写 stderr,同一进程只提示一次。- 边界与验证:残留的 `agent-runner.lock` / `agent-runner.gui-owner.lock` 是 OS 独占句柄锁,进程退出即释放,文件本身不阻塞下次启动;真正阻塞启动的是仍有活进程持锁。验证覆盖 `project_lock_recovery` 6 条(死 PID、空锁宽限、PID 复用时间推断、PID 复用身份不一致、身份一致不抢锁、新鲜空锁不抢锁)、`diagnostic_log` 5 条,以及真实二进制双实例:第二个实例写入 `startup.runner.owner-lock.failed` 并弹出可见提示。[nit] This paragraph says
project_lock_recoveryhas 6 cases and omits the unrepresentable-PID test.decision-log.mdon the same change correctly says 7, and the new module actually has 7 tests (reclaims_unrepresentable_owner_pidplus the six listed here).Suggestion: Align the technical note with the 7 tests, including the
u64::MAXpid case.@@ -2331,0 +2410,4 @@// 配置目录确定之前先按标识符推导 APPDATA 下的日志路径,确定后再切到真实配置// 目录,保证 `configure_game_creator_runtime_config_dir` 自身失败也有落点。let startup_log = Arc::new(StartupLogSlot::new(std::env::var_os("APPDATA")[suggestion] The early slot is only
APPDATA/<identifier>/diagnostics/startup.log. That matches Windows Tauriapp_config_dir(%APPDATA%\world.genarrative.ai-game-creator), which is why the dual-instance owner-lock dialog works there. It is still a dead diagnostic path whenAPPDATAis unset:StartupLogSlot::failno-ops without a path, andconfigure_game_creator_runtime_config_dir’sinspect_erris itself gated onsetup_log.path(). Non-Windows and stripped-env Windows therefore still get no log and no dialog for the one failure this fallback was added to catch.--config-diris also applied before the slot is created, but the initial path ignores that already-known directory, so a configure failure under a custom config dir is recorded under APPDATA (if any) rather than the dir the process is actually using.Suggestion: Seed the slot from
game_creator_runtime_config_dir()when it is already set, otherwise from the platform config root (APPDATA on Windows,dirs/XDG_CONFIG_HOME/~/Library/Application Supportelsewhere).fail()should still surface a dialog even if the path is unknown (message without a file), instead of going silent.@@ -129,2 +238,3 @@.map(|elapsed| elapsed.as_secs()).and_then(|modified| modified.duration_since(UNIX_EPOCH).ok()).map(|duration| duration.as_secs()).unwrap_or_default()[suggestion] Age and the old-lock PID-reuse heuristic now treat mtime as an absolute Unix timestamp, but
project_write_lock_file_modified_secondsmaps a missing/pre-epoch mtime to0. The previousmodified().elapsed()fallback was0seconds of age (do not reclaim). The new default is Unix epoch, sounix_timestamp() - 0is huge: an unreadable mtime on an empty/corrupt body reclaims immediately, and an old-format live lock with pid but nocreatedAttakeslock_created_at = 0and thenactual > 5, which looks like PID reuse and steals a live holder. Normalcreate_newcrash leftovers have a real mtime, so this is a fallback inversion rather than the main #310 path.Suggestion: Keep a distinct “mtime unknown” value. Unknown age should stay 0 (do not reclaim); unknown
lock_created_atshould not satisfyactual > lock_created_at + 5.@@ -149,2 +262,2 @@if let Some(owner_alive) = owner_pid.and_then(project_write_lock_process_is_alive) {return !owner_alive;let created_at = project_write_lock_owner_created_at(path);let Some(owner_pid) = project_write_lock_owner_pid(path) else {[bug]
project_write_lock_can_be_reclaimedis only a snapshot observation, butacquire_project_write_lockstill unlinks the path unconditionally after this returns true (andremove_fileNotFoundis treated as a hard failure). The lock file is a closed marker, not an OS hold, so the window is real: process B decides a stale/empty lock is reclaimable, process C deletes it and installs a live payload, then B'sremove_filedeletes C's live lock. Two instances recovering the same crashed project — the actual #310 restart shape — now hit this path after 30s of empty-lock grace (and on PID-reuse reclaim), whereas Drop already refuses to delete on content mismatch. The helper also re-reads the file separately forcreatedAt,pid, andprocessStartedAt, so a replacement between those reads can mix a dead pid with a new inode and still return true.Suggestion: Parse the payload once, and only unlink if the bytes/nonce still match that snapshot; treat
NotFoundas “already gone” and retrycreate_newinstead of returning清理失效项目写锁失败. The same-process Drop path already has the content-match pattern to copy.@@ -151,0 +274,4 @@// 新锁自带启动身份:同一进程的身份恒定,不一致即为 PID 复用。(Some(stored), Some(actual)) => stored != actual,// 旧锁没有启动身份,只能用“启动时间晚于锁创建时间”推断 PID 复用。(None, Some(actual)) => {[suggestion] The claimed contract is “live holder is never stolen” and “unknown-alive still waits 600s”, but the new tests do not cover those on the old-payload path.
project_write_lock_keeps_matching_process_identityonly feeds a matchingprocessStartedAt. The createdAt+5s heuristic at this line is the one that can false-positive (clock rollback, or the mtime-epoch fallback in Issue 2), and nothing asserts that a live process whose start time is beforecreatedAtkeeps the lock. There is also no case whereprocess_is_alivereturnsNoneand a fresh lock must survive until 600s.Suggestion: Add an old-format fixture (pid + createdAt, no
processStartedAt) whose live child started beforecreatedAt, expecting contention; and an unknown-liveness fixture that is younger than 600s, also expecting contention.@@ -0,0 +105,4 @@}/// 复现 A:崩溃发生在 create_new 成功、payload 写盘之前,留下 0 字节锁。/// 现在要等满 600 秒才会回收,重启后 10 分钟内所有写操作都失败。[suggestion] Several new comments narrate the pre-fix bug as current behavior, or over-claim the code. This test still says “现在要等满 600 秒才会回收”; the PID-reuse test says “只要那个进程还活着,残留锁就永远不会被回收”. After merge those sentences are false, so the next reader will think the tests document live product rules. In
filesystem.rs:264the empty-body branch says missing pid “只可能是” a crash betweencreate_newand payload write; that branch also covers unreadable files, non-JSON, and JSON without a numericpid.Suggestion: State the invariant the test now enforces (“empty body older than the 30s grace is reclaimed”), and drop the “只可能是” claim — missing pid is “no usable holder”, not a proven crash.
@@ -1333,0 +1335,4 @@- `.agent/project.lock` 新增 `processStartedAt`(持有进程启动时间,Unix 秒)。PID 仍存活时必须先核对启动身份:身份不一致即判定 PID 复用,可直接回收;旧锁没有该字段时退回“进程启动时间晚于锁 `createdAt` 加 5 秒容差”的推断。崩溃停在 `create_new` 与落盘 payload 之间的空锁 / 坏锁宽限期从 600 秒收紧到 30 秒;无法判定持有者是否存活时继续按 600 秒保守回收,活持有者仍然不回收。- 启动诊断日志改为 `StartupLogSlot`:`configure_game_creator_runtime_config_dir` 之前按应用标识符推导 APPDATA 路径,成功后再切换到真实配置目录,`startup.*.failed` 与 `show_startup_error_dialog` 不再是死分支。Windows 启动失败恢复系统消息框并附诊断日志路径,其它平台写 stderr,同一进程只提示一次。- 边界与验证:残留的 `agent-runner.lock` / `agent-runner.gui-owner.lock` 是 OS 独占句柄锁,进程退出即释放,文件本身不阻塞下次启动;真正阻塞启动的是仍有活进程持锁。验证覆盖 `project_lock_recovery` 6 条(死 PID、空锁宽限、PID 复用时间推断、PID 复用身份不一致、身份一致不抢锁、新鲜空锁不抢锁)、`diagnostic_log` 5 条,以及真实二进制双实例:第二个实例写入 `startup.runner.owner-lock.failed` 并弹出可见提示。[nit] This paragraph says
project_lock_recoveryhas 6 cases and omits the unrepresentable-PID test.decision-log.mdon the same change correctly says 7, and the new module actually has 7 tests (reclaims_unrepresentable_owner_pidplus the six listed here).Suggestion: Align the technical note with the 7 tests, including the
u64::MAXpid case.结论
上次指出的阻塞问题已在
c0ef876处理完毕:remove_file的 NotFound 当硬失败Option区分「未知」和「纪元 0」:未知年龄不回收,未知创建时间不做 PID 复用推断--config-dir),否则退到平台配置根;路径未知时fail()仍给出用户可见提示当前没有阻塞性正确性缺陷,通过。
评审意见处理(
c0ef876b1)感谢评审。6 条意见全部已处理,逐条对应:
🐞 bug:回收是 check-then-
remove_file已改为“单次快照 + 内容核对后删除”:
ProjectWriteLockSnapshot,一次读入字节并解析pid/createdAt/processStartedAt,不再分别重读三个字段;project_write_lock_reclaim在 unlink 前重新fs::read并逐字节比对快照,内容已被并发方替换或文件已消失时返回false,调用方重试create_new;remove_file的NotFound不再当成硬失败(不再报清理失效项目写锁失败)。新增
project_write_lock_reclaim_skips_replaced_or_removed_lock_file直接钉住这条:判定后被并发方替换成自己的活锁 → 不删除;文件已消失 → 不报错。suggestion:mtime 回退反转
project_write_lock_file_modified_seconds改为返回Option<u64>,判据改成接收modified_at: Option<u64>:年龄未知 → 不回收;旧锁lock_created_at未知 → 不再满足actual > lock_created_at + 5。新增project_write_lock_decision_keeps_lock_when_mtime_is_unknown,同时保留“mtime 已知且超过宽限期即回收”的对照断言,避免把保守改成永不回收。suggestion:旧 payload 路径缺测试
补两条:
project_write_lock_keeps_live_old_format_holder_started_before_created_at:真实活进程 +pid/createdAt、无processStartedAt,且进程启动时间早于createdAt→ 期望占用;project_write_lock_decision_keeps_young_lock_when_liveness_is_unknown:存活查询返回None的年轻锁 → 期望占用,600 秒后 → 期望回收。这条走判据函数的注入参数,因为真实进程无法稳定产生None。suggestion:
APPDATA缺失时启动诊断仍是死路径StartupLogSlot::fail不再依赖路径:始终show_startup_error_dialog(self.path().as_deref()),路径未知时弹不带文件的消息框(Windows)/ 写 stderr(其它平台);inspect_err去掉if let Some(path) = setup_log.path()门禁;early_startup_log_path:优先用已生效的配置目录(含--config-dir),否则退到平台配置根(WindowsAPPDATA、macOSApplication Support、其它平台XDG_CONFIG_HOME/~/.config);early_startup_log_path_prefers_the_already_applied_config_dir与startup_log_slot_fail_without_path_still_reports_instead_of_going_silent。check-config.mjs的启动诊断守卫已同步到新契约(fail的 append + 无条件可见提示、early_startup_log_path存在),避免守卫被绕过。suggestion:注释叙述修复前的缺陷
空锁用例改为陈述不变式(“空锁超过 30 秒宽限期即回收”),PID 复用两条改为“旧格式按时间推断 / 新格式按启动身份”,非法进程号用例改为陈述判据;
filesystem.rs空锁分支去掉“只可能是”,改为“没有可用的持有者信息(空锁、坏锁、无数字 pid 的锁)”。nit:技术方案文档写 6 条
已改为与
decision-log一致的 11 条,并列出新增用例。验证
c0ef876b1):4 个 job 全绿,Native shell tests成功;bin 测试2350 passed; 0 failed; 15 ignored,project_lock_recovery11 条在 Linux runner 上全部通过。project_lock_recovery11 passed / 0 failed;diagnostic_log7 passed(含新增 2 条)。cargo fmt --check、npm run ai-game-creator-shell:typecheck(含check-config.mjs)、npm run check:encoding(4329 文件)、git diff --check通过。