Skip to content

Keep SSH remote job running when its PTY session hangs up - #70573

Merged
potiuk merged 1 commit into
apache:mainfrom
jason810496:fix/ssh-remote-job-detach-stdin
Jul 28, 2026
Merged

Keep SSH remote job running when its PTY session hangs up#70573
potiuk merged 1 commit into
apache:mainfrom
jason810496:fix/ssh-remote-job-detach-stdin

Conversation

@jason810496

Copy link
Copy Markdown
Member

SSHRemoteJobOperator launches the remote job detached under setsid so it survives the SSH connection dropping. The wrapper redirected the job's stdout/stderr to /dev/null but left its stdin on the launching terminal. A fresh setsid session leader that still holds a terminal on any fd re-adopts it as its controlling terminal, so when an SSH session that allocated a PTY (get_pty=True) hangs up, the detached job receives SIGHUP and dies — orphaning the work the operator exists to keep alive.

Redirecting stdin from /dev/null as well (standard daemonization) leaves the job in a session with no controlling terminal, immune to the hangup.

This is also the root cause of the flaky test_kill_terminates_whole_job_tree_under_job_control: its PTY harness hangs up the terminal and then asserts the job is still running. The job was being killed by that hangup, which the @pytest.mark.flaky(reruns=5) marker only masked. With the wrapper fixed the test is deterministic, so the marker and its stale "process-group race" comment are removed.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.8)

Generated-by: Claude Code (Opus 4.8) following the guidelines

SSHRemoteJobOperator launches the remote job detached under setsid so it
survives the SSH connection dropping. The launcher redirected the job's stdout
and stderr to /dev/null but left its stdin on the launching terminal. A fresh
setsid session leader that still holds a terminal on any file descriptor
re-adopts it as its controlling terminal, so when an SSH session that allocated
a PTY hangs up, the job received SIGHUP and died -- orphaning the work the
operator exists to keep alive. Detaching stdin as well leaves the job in a
session with no controlling terminal, immune to the hangup.

@jason810496 jason810496 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some PRs ( #69933 ) still encountered the CI failure of SSH provider even after rebasing on top of #70562.

cc @potiuk

@potiuk
potiuk merged commit c04b93b into apache:main Jul 28, 2026
83 checks passed
@1fanwang

1fanwang commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Thanks for tracking this down, @jason810496. landed on the same RC while chasing this:

The detached job keeps the launcher's terminal on fd 0, so the fresh  setsid  session leader re-acquires it as its controlling terminal. When the pty hangs up, the job gets SIGHUP and dies. Redirecting stdin from  /dev/null  is what fully detaches it, exactly as you have it on both the  setsid  and  nohup  paths.

I reproduced it in a Linux container. The controlling-terminal timing makes it intermittent on CI but deterministic in a slower Docker VM, which gives a clean before/after.

  1. Baseline (current main): the real test fails with the CI assertion, all 5 reruns exhausted

Linux 6.10.14-linuxkit aarch64
$ pytest test_remote_job.py::TestPosixKillBehaviour::test_kill_terminates_whole_job_tree_under_job_control --noconftest

    job_pids = subprocess.run(
        ["pgrep", "-f", marker], capture_output=True, text=True, check=False
    ).stdout.split()
  assert job_pids, "job never started"

E AssertionError: job never started
E assert []

test_remote_job.py:426: AssertionError
=========================== short test summary info ============================
FAILED test_remote_job.py::TestPosixKillBehaviour::test_kill_terminates_whole_job_tree_under_job_control
========================== 1 failed, 5 rerun in 0.32s ==========================

  1. Mechanism: the detached job holds the pty as its controlling terminal

 ps  on the job right before the pty hangup.  pid == sid == pgid  confirms  setsid(2)  gave it a new session;  tty = pts/0  shows it still holds the launcher's pty as its controlling terminal, which is the path the SIGHUP travels down:

recorded pid: 31
ps (pid sid pgid tty) BEFORE hangup:
31 31 31 pts/0
job alive AFTER hangup: False

  1. With this PR's change: the test passes deterministically, no regression

Reproduce

From an apache/airflow checkout. The test skips on macOS, so run it under Linux/Docker:

docker run --rm -v "$PWD:/repo" -w /tmp python:3.10 bash -c '
apt-get update -qq && apt-get install -y -qq util-linux procps >/dev/null
pip install -q pytest pytest-rerunfailures
cp /repo/providers/ssh/src/airflow/providers/ssh/utils/remote_job.py .
cp /repo/providers/ssh/tests/unit/ssh/utils/test_remote_job.py .
sed -i "s/from airflow.providers.ssh.utils.remote_job import (/from remote_job import (/" test_remote_job.py
pytest test_remote_job.py::TestPosixKillBehaviour::test_kill_terminates_whole_job_tree_under_job_control --noconftest'

Dropping the  @pytest.mark.flaky(reruns=5)  marker is the right call too: with stdin detached the test passed 20/20 for me, so it no longer needs reruns. +1 from me.

@jason810496

Copy link
Copy Markdown
Member Author

Yeah, I verified on Linux container as well. Having standard daemonization solves the flaky test issue. Thanks for double checking this!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants