Skip to content

Commit 1eaf801

Browse files
committed
ControlModeEngine(abstraction): Engine hooks for probe/switch
Clean up abstraction leaks by replacing isinstance(ControlModeEngine) checks with hook methods in the base Engine class: - Add probe_server_alive() hook: allows engines to check server liveness without bootstrapping (ControlModeEngine does direct subprocess probe) - Add can_switch_client() hook: allows engines to indicate if switch-client is meaningful (ControlModeEngine checks for non-control clients) - Update Server.is_alive() to use probe hook instead of isinstance - Update Server.raise_if_dead() to use probe hook instead of isinstance - Update Server.switch_client() to use hook directly (was via getattr) - Fix thread join in close() to skip current thread during GC This removes all isinstance(ControlModeEngine) checks from Server, making the Engine abstraction clean and extensible.
1 parent 1a76783 commit 1eaf801

File tree

3 files changed

+88
-18
lines changed

3 files changed

+88
-18
lines changed

src/libtmux/_internal/engines/base.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,39 @@ def iter_notifications(
161161
return
162162

163163
# Optional hooks ---------------------------------------------------
164+
def probe_server_alive(
165+
self,
166+
server_args: tuple[str | int, ...],
167+
) -> bool | None:
168+
"""Probe if tmux server is alive without starting the engine.
169+
170+
Returns
171+
-------
172+
bool | None
173+
True if server is alive, False if dead, None to use default check.
174+
Return None to fall back to running ``list-sessions`` via the engine.
175+
176+
Notes
177+
-----
178+
Override in engines that shouldn't start on probe (e.g., ControlModeEngine).
179+
"""
180+
return None
181+
182+
def can_switch_client(
183+
self,
184+
*,
185+
server_args: tuple[str | int, ...] | None = None,
186+
) -> bool:
187+
"""Check if switch-client is meaningful for this engine.
188+
189+
Returns
190+
-------
191+
bool
192+
True if there is at least one client that can be switched.
193+
Default implementation returns True (assumes switching is allowed).
194+
"""
195+
return True
196+
164197
@property
165198
def internal_session_names(self) -> set[str]:
166199
"""Names of sessions reserved for engine internals."""

src/libtmux/_internal/engines/control_mode.py

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,20 @@ def close(self) -> None:
169169
self._server_args = None
170170
self._protocol.mark_dead("engine closed")
171171

172-
# Join threads to ensure clean shutdown (non-daemon threads)
173-
if self._reader_thread is not None and self._reader_thread.is_alive():
172+
# Join threads to ensure clean shutdown (non-daemon threads).
173+
# Skip join if called from within the thread itself (e.g., during GC).
174+
current = threading.current_thread()
175+
if (
176+
self._reader_thread is not None
177+
and self._reader_thread.is_alive()
178+
and self._reader_thread is not current
179+
):
174180
self._reader_thread.join(timeout=2)
175-
if self._stderr_thread is not None and self._stderr_thread.is_alive():
181+
if (
182+
self._stderr_thread is not None
183+
and self._stderr_thread.is_alive()
184+
and self._stderr_thread is not current
185+
):
176186
self._stderr_thread.join(timeout=2)
177187

178188
def __del__(self) -> None: # pragma: no cover - best effort cleanup
@@ -312,6 +322,31 @@ def internal_session_names(self) -> set[str]:
312322
return set()
313323
return {self._internal_session_name}
314324

325+
def probe_server_alive(
326+
self,
327+
server_args: tuple[str | int, ...],
328+
) -> bool | None:
329+
"""Check if tmux server is alive without starting control mode.
330+
331+
Performs a direct subprocess check to avoid bootstrapping the control
332+
mode connection just to probe server liveness.
333+
334+
Returns
335+
-------
336+
bool
337+
True if server is alive (list-sessions returns 0), False otherwise.
338+
"""
339+
tmux_bin = shutil.which("tmux")
340+
if tmux_bin is None:
341+
return False
342+
343+
result = subprocess.run(
344+
[tmux_bin, *[str(a) for a in server_args], "list-sessions"],
345+
check=False,
346+
capture_output=True,
347+
)
348+
return result.returncode == 0
349+
315350
def exclude_internal_sessions(
316351
self,
317352
sessions: list[Session],

src/libtmux/server.py

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,14 @@ def is_alive(self) -> bool:
212212
>>> tmux = Server(socket_name="no_exist")
213213
>>> assert not tmux.is_alive()
214214
"""
215-
# Avoid spinning up control-mode just to probe.
216-
from libtmux._internal.engines.control_mode import ControlModeEngine
215+
server_args = tuple(self._build_server_args())
217216

218-
if isinstance(self.engine, ControlModeEngine):
219-
return self._probe_server() == 0
217+
# Use engine hook to allow engines to probe without bootstrapping.
218+
probe_result = self.engine.probe_server_alive(server_args)
219+
if probe_result is not None:
220+
return probe_result
220221

222+
# Default: run list-sessions through the engine.
221223
try:
222224
res = self.cmd("list-sessions")
223225
except Exception:
@@ -234,23 +236,24 @@ def raise_if_dead(self) -> None:
234236
... print(type(e))
235237
<class 'subprocess.CalledProcessError'>
236238
"""
237-
from libtmux._internal.engines.control_mode import ControlModeEngine
239+
server_args = tuple(self._build_server_args())
238240

239-
if isinstance(self.engine, ControlModeEngine):
240-
rc = self._probe_server()
241-
if rc != 0:
241+
# Use engine hook to allow engines to probe without bootstrapping.
242+
probe_result = self.engine.probe_server_alive(server_args)
243+
if probe_result is not None:
244+
if not probe_result:
242245
tmux_bin_probe = shutil.which("tmux") or "tmux"
243246
raise subprocess.CalledProcessError(
244-
returncode=rc,
245-
cmd=[tmux_bin_probe, *self._build_server_args(), "list-sessions"],
247+
returncode=1,
248+
cmd=[tmux_bin_probe, *server_args, "list-sessions"],
246249
)
247250
return
248251

252+
# Default: run list-sessions through the engine.
249253
tmux_bin = shutil.which("tmux")
250254
if tmux_bin is None:
251255
raise exc.TmuxCommandNotFound
252256

253-
server_args = self._build_server_args()
254257
proc = self.engine.run("list-sessions", server_args=server_args)
255258
if proc.returncode is not None and proc.returncode != 0:
256259
raise subprocess.CalledProcessError(
@@ -483,10 +486,9 @@ def switch_client(self, target_session: str) -> None:
483486

484487
server_args = tuple(self._build_server_args())
485488

486-
# If the engine knows there are no "real" clients, mirror tmux's
487-
# `no current client` error before dispatching.
488-
can_switch = getattr(self.engine, "can_switch_client", None)
489-
if callable(can_switch) and not can_switch(server_args=server_args):
489+
# Use engine hook to check if switch-client is meaningful.
490+
# For control mode, this ensures there is at least one non-control client.
491+
if not self.engine.can_switch_client(server_args=server_args):
490492
msg = "no current client"
491493
raise exc.LibTmuxException(msg)
492494

0 commit comments

Comments
 (0)