Repository navigation
[Factory Experiment] fix(proxy): join spawned tasks before cleanup to prevent orphaning - #163
timanglade wants to merge 2 commits into
Conversation
timanglade
changed the title
|
I don't seem to have permissions to assign reviewers anymore, but pinging @ejj-agent to see if I can get it to review this PR via a comment. cc @ejj. |
ejj-agent
left a comment
ejj-agent
left a comment
There was a problem hiding this comment.
AI-generated review draft. This has not been reviewed by a human. Any comments made are non-binding; feel free to ignore them by resolving. This uses the same AI-review process Ethan uses to review his own code.
Overall review body
I reviewed the proxy lifecycle change in src/proxy/mod.rs for correctness, simplicity, diff hygiene, and tests. I did not find any substantive concerns worth posting: the diff is narrowly scoped, the cancellation/join ordering matches the intended cleanup sequencing for the DNS and local acceptor tasks, and the existing targeted unit test still passes.
Checks run locally on the PR head: cargo test test_parse_connect_target.
dc0a77e to
247b78d
Compare
247b78d to
2942493
Compare
When run_external_acceptor exits (stream end or cancellation), defer!(nftables_clean()) used to fire immediately, removing nftables rules before the spawned DNS handler and local acceptor tasks could observe cancellation. Now we cancel and join both spawned tasks before returning, so nftables rules stay in place until all handlers stop.
|
Sorry for the noise, agent messed up the rebase due to a different factory experiment having created a branch with the same name, Should be all good now. |
Reverted all source and interface doc renames back to modular structure (src/*/mod.rs, context/interfaces/src/*/mod.md). Restored .claude/CLAUDE.md. The proxy fix in src/proxy/mod.rs is preserved: clones CancellationToken before moving into run_external_acceptor, stores JoinHandles for spawned DNS and local acceptor tasks, and calls cancel.cancel() + tokio::join! before returning to ensure nftables_clean() only fires after handlers stop.
|
I'm not sure why Gas Town picked up this PR again and started doing more work on it when it did; another good reason to avoid it for now I guess. Closing this PR. |
2 participants
Part of an experiment with setting up an automated software factory. Authored by Gas Town on Kilo, using DeepSeek V4 Pro / DeepSeek V4 Flash running on Fireworks. The factory was only fed the content of IM-67, then left completely unsupervised.
Summary
JoinHandles for the spawned DNS handler and local acceptor tasks inHandle::runrun_external_acceptorreturns, cancel the token and join both spawned tasksdefer!(nftables_clean())only fires after all handlers have stoppedDetails
Previously,
run_external_acceptorconsumed thecanceltoken. When it exited (stream end or cancellation),defer!(nftables_clean())fired immediately — before the spawned DNS and local acceptor tasks could observe the daemon'scancel.cancel(). This left a window where nftables rules were gone but listeners were still active.The fix clones
cancelforrun_external_acceptor, then callscancel.cancel()andtokio::join!on both handles before returning. This guarantees all handlers have stopped before nftables cleanup runs.