test(daemon): retire the listen socket with its owner - #1833
Conversation
SIGKILL skips the daemon's endpoint unlink. A child that inherited the listen descriptor still accepts on that path, so the next spawn reports a live daemon. branch_search init does exactly that: spawn, init, drop, spawn again. Waiting for the path to go quiet papers over the leak. Kill the owner's process group, then unlink the socket the owner bound. A path that still accepts is a daemon this call did not stop. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3034d5ad79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if terminate_and_reap(&mut self.child).is_ok() { | ||
| self.retire_owned_unix_endpoint(); |
There was a problem hiding this comment.
When wait_for_exit reaps a daemon, owned_unix_endpoint remains set. Existing graceful-restart paths such as indexing_lifecycle_test.rs:375-384 then assign daemon = spawn_tracedecay_daemon_with(...); the replacement is spawned and binds the same path before the overwritten guard is dropped, so this Drop unlinks the replacement daemon's live socket and subsequent requests cannot connect. Retire the endpoint when try_wait/wait_for_exit observes an exit, or otherwise ensure it is cleared before a replacement is spawned.
Useful? React with 👍 / 👎.
|
Cancelled CI run 35427490131: this fixes the same defect as f2ab808 + 8b75523 (clone-postings occurrence indexes, per-page resume reads) on |
|
Correction to my previous comment: this PR is not a duplicate of the clone-postings fix; that text was meant for #1824/#1830. I cancelled the run only to keep the shared runner queue (2 concurrent, ~20 deep) moving while #1797 waits; this PR gets its own review agent and a fresh dispatch afterwards. Apologies for the noise. |
…n-listen-endpoint-464e
The restart journeys reassign the guard: `daemon = spawn_...` evaluates the RHS first, so the successor binds `.tracedecay/daemon.sock` before the predecessor guard is dropped. A predecessor that already exited on SIGTERM had unlinked its own socket, so the unconditional retire in `Drop` removed the *successor's* live path. Every later connect in `ignored_dependency_admission_survives_physical_daemon_restart_without_widening` and `mounted_incremental_lifecycle_preserves_only_complete_compatible_generations` then failed with ENOENT. Retire the recorded endpoint only when this call force-stopped a live child. A child that exited on its own already ran its own cleanup, so there is nothing this guard still owns. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Folded into integration batch #1848 (branch ci/pr-batch-c, CI run 35431957131) after the lane review verdict MERGE with fixes pushed here. This PR is merged once that run is green. |
tests/common/mod.rs takes #1819's harness wholesale (publisher registry for socket release, reap tracking) over #1833's; #1833's regression test now records its socket through release_socket_on_stop. In publication_store.rs, #1819's commit_observed_pointer keeps #1805's require_regular_pointer_slot check ahead of the read and maps the rename error through map_pointer_io so directory faults stay in the publication family. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 participants
Summary
branch_search_serves_a_committed_generation_behind_dirty_worktree_state(tip3f71bccb2dba) failed because init drops its daemon and the following spawn immediately treats an inherited listen descriptor as a live daemon.Motivation
Push CI on
bc2ced4882markedbranch_search_serves_a_committed_generation_behind_dirty_worktree_stateflaky:refusing to replace a live test daemon. The follow-up waited up to 10s for the path to stop accepting. That treats the leak as a race.tracedecay daemon rununlinks its socket on SIGTERM, before drain. The harness force-stops with SIGKILL, which skips that cleanup. A child that inherited the listen descriptor acrossforkkeeps accepting on the same path after the leader is reaped.init_project_fixturethen spawns the replacement and the instantaneous check reports a live daemon.Changes
terminate_and_reapsignals the leader's process group, while that pid is still the live leader, then reaps the leader. Signaling a recycled pid is avoided.reaped_owner_releases_an_inherited_listen_socketbinds a socket, forks a child that holds the descriptor, drops the owner, and asserts the path neither exists nor accepts..github/workflows/ci.ymlis unchanged. Nopull_requesttrigger was added.Test plan
cargo test -p tracedecay --features test-transport --test transport_acceptance_suite daemon_fault_harness_test::reaped_owner_releases_an_inherited_listen_socket -- --exact—ok. 1 passed; 0 failed; 0 ignored; 0 measured; 12 filtered outChecklist
.envfiles includedCHANGELOG.mdupdated (test harness only; no user-facing change)