Conversation
6d8f049 to
31fa82d
Compare
From review of #23. `_daemon_lock` was a local of `run()`'s future, so the lock went with the future — not "for the process's life", as its own doc claimed. The three tickers `serve` spawns are never joined and `crew-transcript` flushes every 150ms, so the next daemon could take the lock and load transcripts this one was still writing. It is forgotten deliberately now, with the reason written where the claim was. Every non-`EWOULDBLOCK` errno was read as "another daemon holds it", so a home on a filesystem without locks reported `daemon already running` forever, with no daemon anywhere and a message pointing at `crew stop`. Only `EWOULDBLOCK` means someone else has it. `cleanup` asked the socket whether the daemon was gone, and the socket cannot answer that: it is absent while a daemon opens its roster and again while it tears down. It asks the process, the way `ui_is_live` does. That also fixes what the socket check broke — `stop_daemon` returning `Ok(())` having stopped nothing, after which `ensure_daemon` pointed the caller at a daemon already draining its agents. A stop that did not stop is an error, and `ensure_daemon` propagates it. `wait_dead` waits on the process too, and for eight seconds: `shutdown_agents` seals every transcript and kills every agent, which outlasts a two-second guess on a real roster. The pid is written when the lock is taken. Both are the same fact, and writing it after the roster opened left the holder unnameable — so unstoppable — for the seconds that took. A socket that answers still bails, lock or no lock. Nothing enforces that `crew.lock` survives a tidied home or a restored backup, and a fresh inode carries its own lock; one connect keeps the old guard. `serve` is split out so the roster is torn down however boot ends. Bind and the version write return through `?` after every agent is open, and this PR named that orphaning as a reason for the lock without fixing it. `ensure_daemon` reaps the daemon it spawns and stops waiting when it exits. The lock refuses a second daemon outright, so exiting in milliseconds is the common case now: the wait burned its whole four seconds for a child that was already gone, and left a zombie per attempt under the desktop process.
cf92d3f to
becfa9d
Compare
|
4차 리뷰 결과 머지 보류. 라운드마다 통합 지점이 새로 깨지는 패턴이 확정적이에요. 가장 문제인 지적: 락이 영구 brick 을 만들 수 있어요. 데몬이 소켓이 죽은 뒤 기존 동작은 10회에 1회 레이스이고 자기 치유돼요. 이 PR 은 그걸 드물지만 복구 불가인 실패로 바꿔요. 나쁜 트레이드예요. 나머지 지적도 전부 맞아요:
근본은 이래요: 락을 넣으면 소켓과 수명이 다른 두 번째 소유권 신호가 생기고, 기존 흐름 전부( |
b056021 to
80093a9
Compare
The socket was answering two different questions. `daemon::run` asked it whether a daemon existed, and the CLI asked whether one was answering; those are not the same thing, and the first cannot be asked that way. Connecting and then acting on the answer left the whole roster open in between, long enough for a second daemon to pass the same check, unlink the first's live socket — `remove_stale_socket` never asked whether anyone was on it — bind its own, and spawn every agent onto the same cwd and transcripts. Ten paired starts against the current binary put both of them in "daemon listening" once. Ownership is a lock now, and the socket keeps only the question it can answer. A POSIX record lock rather than `flock`, for two things `flock` cannot do: `F_GETLK` asks who holds it without taking it, so a caller can poll without becoming the answer, and it reports the holder's pid. That pid is not always one to trust. `F_GETLK` answers `-1` for an open-file-description lock, and a remote holder over NFS can answer `0` — which `kill` reads as "my process group" and "every process I may signal". `Owner` therefore distinguishes held-by-this-pid from held-by-nobody-nameable, and nothing unnameable is ever signalled. A probe must not create the lock file either: asking about a fresh inode answers "nobody" whoever holds the one that was there. Only the daemon taking ownership creates it, and `cleanup` asks the socket as well as the lock, so a lock file deleted under a running daemon cannot lead to clearing its files. The split is what lets `ensure_daemon` tell the three states apart. A daemon that owns the home gets the time a roster open takes to answer; one that owns it and stays quiet is wedged and has to let go first. `stop_daemon` asks, then SIGTERMs, then SIGKILLs, because the lock is released when the process ends and ending it is the whole job — without that last step a daemon stuck in `shutdown_agents` would hold the home for good. It asks first, and with the same budget the start path gets, because signalling skips the teardown that seals transcripts. How long a roster takes to open depends on the bots, the machine and the CLIs, none of which this can see, so the guess is overridable with `CREW_READY_TIMEOUT`. Too low kills a healthy daemon eighteen seconds into starting. The version is written before the socket is bound. A caller polls for the socket and checks the version the instant it answers, and in the other order that window reads as a mismatch and stops a daemon that had just started correctly. Both paths into `ensure_daemon` share that check, so a caller that lost a start race does not end up talking a newer protocol to somebody else's daemon. Losing that race is not a failure either. The loser's child exits in milliseconds while the winner is still opening its roster, so it keeps waiting for whoever owns the home and only gives up once nobody does. Bind stays where it was, so a live socket goes on meaning "answering" and the `is_socket_live()` gates in the CLI keep reading it that way — none of them change. `read_pid` is gone; the lock answers that now. There is no fallback to the old check. A home where ownership cannot be asked about is a home where two daemons cannot be kept apart, and saying so beats going quiet and corrupting a roster. `serve` is split out so the roster is torn down however boot ends: bind and the version write return through `?` with every agent already spawned, orphaning a CLI child per agent onto the transcripts the next daemon would load. The guarantee is tested across processes, which is the only way it can be — a record lock is per-process, so a second lock inside the owner succeeds and an in-process test would pass while the invariant was broken. Those tests caught a bug in this change's own first draft, where reaping the spawned child by killing it took down the daemon the call had just started.
80093a9 to
d6b50c9
Compare
|
6차 리뷰 결과 머지 보류 권고. 두 가지가 결정적이에요. ① 이 설계의 전제가 성립하지 않아요. 즉 "락은 잡혀 있는데 응답 없음 = wedged" 라는 판별이, 가장 흔한 wedge 형태(데드락·SIGSTOP·shutdown_agents 에서 멈춤 — 소켓은 여전히 bound)에서 "정상"으로 읽혀요. 제 통합 테스트가 wedge 를 만들려고 ② 6라운드 중 4번, 제 코드에 심각한 결함이 있었어요.
전부 리뷰나 실측이 잡았고 제가 스스로 잡은 건 없어요. 이번 라운드 나머지 지적도 맞아요: 권고: 닫고, 이 레이스는 알려진 한계로 기록. 발생 대가는 10회에 1회이고 자기 치유돼요. 여섯 번 시도해서 매번 그보다 나쁜 걸 넣었어요. 살릴 가치가 있는 건 |
|
닫습니다. 6라운드 리뷰 후 전제를 다시 측정했고, 제가 틀렸어요. 계측 트레이스: 측정된 피해가 이 PR 을 정당화했던 시나리오와 달라요:
즉 "두 데몬이 모든 에이전트를 같은 transcript 에 spawn"·"orphan 된 CLI 자식" 은 측정에서 재현되지 않아요. 저는 리뷰의 시나리오 서술을 검증 없이 전제로 받아 여섯 번 고쳤고, 그중 네 번은 그보다 나쁜 결함을 넣었어요 ( 레이스는 알려진 한계로 기록하고, 측정으로 남은 유일한 실제 문제 — 패자가 |
TL;DR
소켓이 두 질문에 답하고 있었어요 — "데몬이 있나"(소유권)와 "데몬이 응답하나"(준비됨). 앞의 질문은 소켓으로 물을 수 없어요. 소유권을 락으로 옮겼어요.
설계
소유권 = 락, 준비됨 = 소켓. 분리하니
ensure_daemon이 세 상태를 구분해요:POSIX record lock(
fcntl) —flock이 못 하는 두 가지 때문이에요:F_GETLK이 취득 없이 조회하고(취득하는 probe 는 소유자와 구별이 안 돼요), 보유자 pid 를 줘요.5차 리뷰에서 고친 것 (둘 다 심각, 둘 다 제 코드)
①
kill에 pid 가드가 없었어요.F_GETLK은 OFD 락에-1, NFS 원격 보유자에0을 줄 수 있어요 →kill(0, SIGTERM)은 호출자 프로세스 그룹 전체,kill(-1, SIGKILL)은 사용자의 모든 프로세스. 2차 리비전의pid as i32버그를 다른 형태로 다시 만든 거예요.이름을 댈 수 없는 소유자에겐 절대 시그널을 안 보내요.
② probe 가 락 파일을 새로 만들었어요. 파일이 지워지면 새 inode 에
F_GETLK→ "주인 없음" →cleanup()이 살아있는 데몬의 소켓을 지워요. 이제 데몬만 생성하고,cleanup은 락과 소켓을 둘 다 물어봐요.나머지 반영:
CREW_READY_TIMEOUT노브 — 로스터 open 시간은 봇·기계·CLI 에 달려서 코드가 볼 수 없어요. 낮게 잡으면 18초째 기동 중인 정상 데몬을 죽여요.ESRCH는 원하던 결과로 처리, 그 외 시그널 실패는 에러로.read_pid삭제(락이 답해요), 중복 dev-dependency 삭제, 테스트의 죽은 산술 제거.Dropteardown 가드 — assert 가 터져도 데몬이 락을 들고 남지 않아요.트레이드오프 (명시)
폴백 없어요. 소유권을 물을 수 없는 홈은 두 데몬을 갈라놓을 수 없는 홈이에요. 조용히 예전 방식으로 내려가 로스터를 깨뜨리는 것보다 에러로 말하는 쪽을 골랐어요.
bind는 안 옮겨요 — 락이 배제를 하니 live 소켓은 계속 "응답 중". CLI 의is_socket_live()게이트 ~15곳 무수정.검증
cargo test --locked -p crew— 214 + 3 passeda_second_daemon_is_refused_and_the_first_keeps_serving— 거절 + pid 로 보유자 지목, 첫 번째 도달 가능, stop 후 잔여 없음starting_a_daemon_leaves_one_answeringa_wedged_daemon_is_taken_over— SIGSTOP + 소켓 제거 상태에서 인수 (CREW_READY_TIMEOUT노브도 같이 검증)FAILED. 자식을 kill 로 회수하면 2개FAILEDcargo fmt --check— 변경한 파일 cleanSIGKILL 단계는 테스트로 고정 못 했어요 — SIGSTOP 된 프로세스도 SIGTERM 기본 동작으로 죽어서, 재현엔 uninterruptible 상태가 필요해요. 방어용이고 근거는 커밋에 있어요.
남은 범위 밖
ensure_daemon의 최악 대기가 길어요 (wedge 경로에서 기본값 기준 ~40초).desktop::run()과 sync Tauri command 11곳에서 불려서, Keep the daemon off the main thread. #21 (async command) 과 같이 봐야 해요. 지금도 sync 라 창이 멈춰요 — 이 PR 이 그 시간을 늘려요.crew.log무회전관계
선행 2건 중 두 번째 (첫째 #22 머지됨). #20 은
serve분리가 여기 들어갔으니 불필요 — 머지 후 닫을게요.