Skip to content

fix(connect): close the --tunnel-only forward on SIGTERM and SIGHUP - #1322

Open
kunalgoesbyken wants to merge 1 commit into
railwayapp:masterfrom
kunalgoesbyken:fix/connect-tunnel-only-sigterm
Open

kunalgoesbyken wants to merge 1 commit into
railwayapp:masterfrom
kunalgoesbyken:fix/connect-tunnel-only-sigterm

Conversation

@kunalgoesbyken

Copy link
Copy Markdown

Closes #1319

Problem

railway connect <db> --tunnel-only only closed its tunnel on Ctrl+C. Stopping it any other way (kill, pkill, systemd, closing the terminal) left the ssh -N -L child 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. Only ForwardGuard::drop kills it.
  • run_tunnel_only (src/commands/connect.rs) awaited only tokio::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_only now awaits wait_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_connect is unchanged. It deliberately ignores SIGINT so the database client can use it.
  • No 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 -- --check and cargo clippy --all-targets --all-features, on Windows and for x86_64-unknown-linux-gnu.
  • cargo test: all commands::connect tests pass.

This needs a release/patch label, which I can't set from a fork.

`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
@kunalgoesbyken

Copy link
Copy Markdown
Author

This is a bug fix to railway connect --tunnel-only, so release/patch should fit for the label check. I can't add labels from a fork.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

railway connect --tunnel-only leaves its ssh forward running when stopped by SIGTERM

1 participant