Let a daemon notice it is unreachable. - #26
Merged
Merged
Conversation
Two daemons can start together and both get a socket: one passes the liveness check, the other binds, and then the first runs its own `remove_stale_socket` and binds a second inode at the same path. The daemon that was unlinked has no way to find out. Its listener still works, nothing ever connects to it, and it sits in the accept loop for the life of the machine — holding its pty children and its three tickers, which keep flushing the transcripts the reachable daemon now owns with whole-file writes. So it checks, once a second, that the path still names the socket it bound. `dev` and `ino`, because the path is the thing that got reused. And then it only exits. Nothing there is its to clean: the socket, pid and version name the daemon that took the path, and `shutdown_agents` would unlink `cli-sessions/<id>` — the ids that daemon's bots resume from, whose loss is silent until the next restart. Its pty children end with the process, measured; a headless agent has no process to end. The race itself stays, and is written up in the issue tracker. Closing it needs the liveness check and the bind to be one operation, and the stale-socket unlink that crash recovery depends on is what stops `bind` from being that.
From review of #26. `bound` was read four statements after `bind`, and a theft landing in that window would record the thief's socket as ours — leaving the check comparing it against itself for the life of the process, which is the one state it exists to end. It is read immediately now. It takes two consecutive readings to act. One `stat` can fail for reasons that are not this, and what follows is ending the process. The message says which it is. "Another daemon has it" was asserted on every mismatch, including a path that is simply gone, and sent whoever read the log hunting for a process that does not exist. The decision is `lost_socket`, so the branch that kills the daemon has a test. The comment points at issue #25, where the race is written up — it pointed at the README, where it is not. The exit note no longer claims a headless agent has nothing to end. A turn in flight loses its thread here, so nothing of ours is written, but its CLI child outlives the process. Interrupting it first is worse: `interrupt` lets the turn thread run on into `session_is_dead`, which calls `clear_session` on the very ids this exit is written to protect.
From the second review of #26. Finding nothing at the path one statement after a successful bind has exactly one cause: another daemon unlinked it. The code read that as "the stat failed, leave the check off", which handed the daemon robbed in the widest part of the window the one outcome this exists to prevent — unreachable forever, tickers still writing. It bails there now, before a ticker exists, and the loop below takes a plain identity with no guard and no `expect`. The turn in flight is reaped after all. Not calling `interrupt` was justified here by a `clear_session` it cannot reach: `interrupt` bumps the generation, and `run_turn` returns at its staleness check before the arm that clears the session. So the CLI child ends with us instead of outliving us to write the session the reachable daemon resumes. Measured: the ids still survive the exit. Missed ticks no longer count as readings. `interval` defaults to bursting through a backlog, so two samples a second apart became two in the same microsecond after any stall — which is the one thing the two readings were for. The test covers the counter rather than `Option::ne`: that it takes two in a row, and that finding the socket again starts the count over.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #25
TL;DR
기동 레이스(#25)의 유일하게 지속적인 피해만 막는 최소 장치예요. 레이스 자체는 안 고쳐요.
두 데몬이 동시에 뜨면 한쪽이 상대의 live 소켓을 unlink 하고 같은 경로에 자기 소켓을 bind 할 수 있어요. 지워진 쪽은 그걸 알 방법이 없어요 — 리스너는 정상이고, 아무도 연결하지 않고, accept 루프에 영구히 남아요. 그동안 ticker 3개가 살아있는 데몬이 소유한 transcript 를 whole-file write 로 계속 flush 해요.
그래서 1초에 한 번, 경로가 아직 자기가 bind 한 소켓을 가리키는지 확인해요 (
dev/ino— 재사용되는 건 경로니까요). 두 번 연속 아니면 종료.종료할 때
std::process::exit(1)만 해요. 소켓·pid·버전 파일은 경로를 가져간 데몬을 가리켜요. 그리고shutdown_agents()를 부르면headless::kill→clear_session()이cli-sessions/<id>를 언링크해서 그 데몬 봇들의--resumeid 를 지워요 — #24 를 닫은 이유예요 (측정: main 2→2, #24 2→0).정확히 하면: 진행 중인 헤드리스 턴이 있으면 그 CLI 자식 프로세스는 이 프로세스보다 오래 살아요. 턴 스레드는
exit과 함께 죽으니 우리 파일에는 아무것도 안 써요. 먼저interrupt로 자식을 정리하는 건 더 나빠요 —interrupt는 턴 스레드를 계속 진행시키고, 그 스레드가session_is_dead경로에서clear_session을 부를 수 있어요. 이 종료가 지키려는 바로 그 id 를요.리뷰 반영 (1차)
bound를bind4문장 뒤에 읽어서, 그 창에서 도둑맞으면 검사가 영구 무효화bind직후로 이동None한 번에 종료 — 일시적stat실패도 포함HeadlessInner.child: Option<Child>확인interrupt는 위험해서 안 씀(위 참조)is gone/is a different socket now구분lost_socket()추출 + 테스트bound가 None 이어도 매 틱statflock으로 레이스를 닫자는 제안은 #23 에서 6 리비전 시도 후 닫았어요 — 이유는 그 PR 과 #25 에 기록돼 있어요.제 실패 유형 대조
#20·#23·#24 에서 제가 넣은 결함이 일곱 번이었어요:
cli-sessions/pid/transcript)검증
cargo test --locked -p crew— 216 passedlost_socket테스트 — 같은 소켓/사라짐/다른 inode/다른 devicea_rebound_socket_is_a_different_socket— 뮤테이션: identity 를 상수로 만들면FAILED... is a different socket now; this daemon is unreachablecli-sessions보존: 2개 → 2개 (Tear down a roster a boot abandons. #24 는 2→0. 지난번 제가 안 본 축)crew.pid보존socket=NO pid=NO)cargo fmt --check— 변경한 코드 clean