From 10934dac6b27ace621f6e21c979aa73a1f90a8c6 Mon Sep 17 00:00:00 2001 From: Dmitry Meyer Date: Thu, 20 Nov 2025 13:43:24 +0000 Subject: [PATCH 1/2] Fix SSHAttach.detach() When error logging was added to SSHTunnel.close()[1], it was discovered that SSHAttach.detach() is called multiple times on detach. Although this is harmless as long as this method is idempotent (and it is), this patch ensures that detach() is no-op when called more than one time, removing ERROR level log noise. As a side-effect, detaching SSHAttach that reuses SSHTunnel no longer kills "master" SSHAttach, meaning that one can `dstack attach` multiple times and detach independently (detaching from the "master" attach still kicks out all other attaches). [1]: https://github.com/dstackai/dstack/pull/3296 --- src/dstack/_internal/core/services/ssh/attach.py | 10 ++++++++++ src/dstack/_internal/core/services/ssh/tunnel.py | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/src/dstack/_internal/core/services/ssh/attach.py b/src/dstack/_internal/core/services/ssh/attach.py index d0ad4ac64..91842665b 100644 --- a/src/dstack/_internal/core/services/ssh/attach.py +++ b/src/dstack/_internal/core/services/ssh/attach.py @@ -12,6 +12,7 @@ from dstack._internal.core.services.ssh.client import get_ssh_client_info from dstack._internal.core.services.ssh.ports import PortsLock from dstack._internal.core.services.ssh.tunnel import SSHTunnel, ports_to_forwarded_sockets +from dstack._internal.utils.logging import get_logger from dstack._internal.utils.path import FilePath, PathLike from dstack._internal.utils.ssh import ( default_ssh_config_path, @@ -21,6 +22,8 @@ update_ssh_config, ) +logger = get_logger(__name__) + # ssh -L option format: [bind_address:]port:host:hostport _SSH_TUNNEL_REGEX = re.compile(r"(?:[\w.-]+:)?(?P\d+):localhost:(?P\d+)") @@ -68,6 +71,7 @@ def __init__( local_backend: bool = False, bind_address: Optional[str] = None, ): + self._attached = False self._ports_lock = ports_lock self.ports = ports_lock.dict() self.run_name = run_name @@ -209,6 +213,7 @@ def attach(self): for i in range(max_retries): try: self.tunnel.open() + self._attached = True atexit.register(self.detach) break except SSHError: @@ -219,9 +224,14 @@ def attach(self): raise SSHError("Can't connect to the remote host") def detach(self): + if not self._attached: + logger.debug("Not attached") + return self.tunnel.close() for host in self.hosts: update_ssh_config(self.ssh_config_path, host, {}) + self._attached = False + logger.debug("Detached") def __enter__(self): self.attach() diff --git a/src/dstack/_internal/core/services/ssh/tunnel.py b/src/dstack/_internal/core/services/ssh/tunnel.py index abdb2ba52..bf7413c19 100644 --- a/src/dstack/_internal/core/services/ssh/tunnel.py +++ b/src/dstack/_internal/core/services/ssh/tunnel.py @@ -174,6 +174,11 @@ def open(self) -> None: stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=SSH_TIMEOUT, + # We don't want the ssh process to receive SIGINT from the controlling terminal + # (e.g., when SSHTunnel is used via CLI->Python API's SSHAttach and the dstack CLI + # process is a foreground process group leader). Starting a new session ensures + # that we don't have the controlling terminal at all. + start_new_session=True, ) except subprocess.TimeoutExpired as e: msg = f"SSH tunnel to {self.destination} did not open in {SSH_TIMEOUT} seconds" From 7ac2a16c12af5a56947ee6dd94b3bd2b571086ed Mon Sep 17 00:00:00 2001 From: Dmitry Meyer Date: Thu, 20 Nov 2025 15:32:10 +0000 Subject: [PATCH 2/2] Check if control socket exists start_new_session is POSIX-only (ignored on Windows), and Windows-specific CREATE_NEW_PROCESS_GROUP flag doesn't work as expected (it seems that ssh still receives CTRL_C_EVENT, at least it exits immediately on Ctrl+C event). --- src/dstack/_internal/core/services/ssh/tunnel.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/dstack/_internal/core/services/ssh/tunnel.py b/src/dstack/_internal/core/services/ssh/tunnel.py index bf7413c19..e4f7f276e 100644 --- a/src/dstack/_internal/core/services/ssh/tunnel.py +++ b/src/dstack/_internal/core/services/ssh/tunnel.py @@ -174,11 +174,6 @@ def open(self) -> None: stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=SSH_TIMEOUT, - # We don't want the ssh process to receive SIGINT from the controlling terminal - # (e.g., when SSHTunnel is used via CLI->Python API's SSHAttach and the dstack CLI - # process is a foreground process group leader). Starting a new session ensures - # that we don't have the controlling terminal at all. - start_new_session=True, ) except subprocess.TimeoutExpired as e: msg = f"SSH tunnel to {self.destination} did not open in {SSH_TIMEOUT} seconds" @@ -209,6 +204,11 @@ async def aopen(self) -> None: raise get_ssh_error(stderr) def close(self) -> None: + if not os.path.exists(self.control_sock_path): + logger.debug( + "Control socket does not exist, it seems that ssh process has already exited" + ) + return proc = subprocess.run( self.close_command(), stdout=subprocess.PIPE, stderr=subprocess.STDOUT ) @@ -220,6 +220,11 @@ def close(self) -> None: ) async def aclose(self) -> None: + if not os.path.exists(self.control_sock_path): + logger.debug( + "Control socket does not exist, it seems that ssh process has already exited" + ) + return proc = await asyncio.create_subprocess_exec( *self.close_command(), stdout=subprocess.PIPE, stderr=subprocess.STDOUT )