Repository navigation
fix(connect): close the --tunnel-only forward on SIGTERM and SIGHUP - #1322
Open
kunalgoesbyken wants to merge 1 commit into
Open
kunalgoesbyken wants to merge 1 commit into
kunalgoesbyken wants to merge 1 commit into
Conversation
`railway connect --tunnel-only` only waited on Ctrl+C. SIGTERM and SIGHUP took the default action and ended the process before the forward guard was dropped, and because the ssh child runs in its own process group it was re-parented to init and kept forwarding the local port. Wait on SIGTERM and SIGHUP as well on Unix so every graceful stop drops the guard and kills ssh. Closes railwayapp#1319
Author
|
This is a bug fix to |
This branch has not been deployed
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
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.
Closes #1319
Problem
railway connect <db> --tunnel-onlyonly closed its tunnel on Ctrl+C. Stopping it any other way (kill,pkill, systemd, closing the terminal) left thessh -N -Lchild running under PID 1, still forwarding the local port to the database.Mechanism
spawn_native_ssh_forward(src/commands/ssh/native.rs) puts ssh in its own process group (process_group(0)), so signals sent to railway never reach it. OnlyForwardGuard::dropkills it.run_tunnel_only(src/commands/connect.rs) awaited onlytokio::signal::ctrl_c(). SIGTERM and SIGHUP took the default action and ended the process without unwinding, so the guard was never dropped.Fix
run_tunnel_onlynow awaitswait_for_shutdown_signal(). On Unix that selects over Ctrl+C, SIGTERM and SIGHUP; elsewhere it stays Ctrl+C. Any of them returns normally, the guard drops and ssh is killed, the same as the Ctrl+C path today.run_ssh_connectis unchanged. It deliberately ignores SIGINT so the database client can use it.PR_SET_PDEATHSIG: it is Linux-only and fires when the spawning thread exits, not the process, which is fragile under tokio. SIGKILL is out of scope.Verification
test_shutdown_signal_resolves_on_terminate_and_hangup(Unix): registers the handlers, checks the future is still pending, raises SIGTERM and then SIGHUP, and checks each one resolves it.cargo fmt --all -- --checkandcargo clippy --all-targets --all-features, on Windows and forx86_64-unknown-linux-gnu.cargo test: allcommands::connecttests pass.This needs a
release/patchlabel, which I can't set from a fork.