Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,45 @@ $ uvx --from 'libtmux' --prerelease allow python
_Notes on the upcoming release will go here._
<!-- END PLACEHOLDER - ADD NEW CHANGELOG ENTRIES BELOW THIS LINE -->

### What's new

#### A tmux call can be given a deadline (#757)

{class}`~libtmux.common.tmux_cmd` takes a `timeout`, and
{class}`~libtmux.Server` takes one as the default for every command it
issues, including the listings behind
{attr}`Server.sessions <libtmux.Server.sessions>`,
{attr}`Session.windows <libtmux.Session.windows>` and
{attr}`Window.panes <libtmux.Window.panes>`. Those listings do not go
through {meth}`Server.cmd() <libtmux.Server.cmd>`, so until now a
consumer could not bound them at all.

Without a timeout nothing changes: the default is to wait, as before.

A tmux server can accept a connection and then never reply, and a call
waiting on one never returns. Giving up on it does not stop it either,
so the waiting calls only accumulate, and enough of them will exhaust a
thread pool and stall work aimed at healthy sockets.

On expiry the tmux process is killed and reaped before
{exc}`~libtmux.exc.TmuxTimeout` is raised, so a call that timed out
leaves nothing running behind it. The command may still have taken
effect -- the process was stopped mid-command, and nothing can be said
either way.

{exc}`~libtmux.exc.TmuxTimeout` is deliberately not a
{exc}`~libtmux.exc.LibTmuxException`. The listing accessors read one of
those as "nothing to list", which is correct for a daemon that has not
started and wrong for a server that has stopped answering: a caller
told there are no sessions goes on to create one on a server that
already has them.

```python
server = Server(timeout=5)
server.sessions # raises TmuxTimeout rather than reporting []
server.cmd("kill-server", timeout=30) # per-call override
```

### Documentation

#### Cleaner `from_env` examples (#719)
Expand Down
31 changes: 29 additions & 2 deletions src/libtmux/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,13 +303,33 @@ class tmux_cmd:

$ tmux new-session -s my session

Parameters
----------
tmux_bin : str, optional
Path to the tmux binary. Defaults to the first ``tmux`` on ``PATH``.
timeout : float, optional
Seconds to wait for the command. On expiry the tmux process is
killed and reaped, then :exc:`~libtmux.exc.TmuxTimeout` is
raised. ``None`` waits indefinitely.

Raises
------
:exc:`~libtmux.exc.TmuxTimeout`
``timeout`` elapsed. The command may or may not have taken
effect -- the process was killed mid-command.

Notes
-----
.. versionchanged:: 0.8
Renamed from ``tmux`` to ``tmux_cmd``.
"""

def __init__(self, *args: t.Any, tmux_bin: str | None = None) -> None:
def __init__(
self,
*args: t.Any,
tmux_bin: str | None = None,
timeout: float | None = None,
) -> None:
resolved = tmux_bin or shutil.which("tmux")
if not resolved:
raise exc.TmuxCommandNotFound
Expand All @@ -336,8 +356,15 @@ def __init__(self, *args: t.Any, tmux_bin: str | None = None) -> None:
encoding="utf-8",
errors="backslashreplace",
)
stdout, stderr = self.process.communicate()
stdout, stderr = self.process.communicate(timeout=timeout)
returncode = self.process.returncode
except subprocess.TimeoutExpired:
# Kill and reap before raising. A caller that gives up on an
# unbounded call leaves the child running, so repeated
# timeouts accumulate tmux clients that nothing is waiting on.
self.process.kill()
self.process.communicate()
raise exc.TmuxTimeout(cmd, t.cast("float", timeout)) from None
except FileNotFoundError:
raise exc.TmuxCommandNotFound from None
except Exception:
Expand Down
20 changes: 20 additions & 0 deletions src/libtmux/exc.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,26 @@ class WaitTimeout(LibTmuxException):
"""Function timed out without meeting condition."""


class TmuxTimeout(Exception):
"""A tmux command did not return within its timeout.

Deliberately NOT a :exc:`LibTmuxException`. The listing accessors
absorb one of those as "nothing to list", which is right for a
daemon that has not started and wrong for a server that stopped
answering -- a caller told there are no sessions goes on to create
one on a server that already has them.

Distinct from :exc:`WaitTimeout`, which is a helper giving up on a
condition. This one means the tmux process was killed mid-command,
so nothing can be said about whether it took effect.
"""

def __init__(self, cmd: list[str], timeout: float, *args: object) -> None:
self.cmd = cmd
self.timeout = timeout
super().__init__(f"tmux did not return within {timeout}s: {' '.join(cmd)}")


class VariableUnpackingError(LibTmuxException):
"""Error unpacking variable."""

Expand Down
1 change: 1 addition & 0 deletions src/libtmux/neo.py
Original file line number Diff line number Diff line change
Expand Up @@ -1133,6 +1133,7 @@ def fetch_objs(
proc = tmux_cmd(
*tmux_cmds,
tmux_bin=server.tmux_bin,
timeout=server.timeout,
)

raise_if_stderr(proc, list_cmd)
Expand Down
10 changes: 9 additions & 1 deletion src/libtmux/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,12 @@ def __init__(
on_init: t.Callable[[Server], None] | None = None,
socket_name_factory: t.Callable[[], str] | None = None,
tmux_bin: str | pathlib.Path | None = None,
timeout: float | None = None,
**kwargs: t.Any,
) -> None:
EnvironmentMixin.__init__(self, "-g")
self.tmux_bin = str(tmux_bin) if tmux_bin is not None else None
self.timeout = timeout
self._windows: list[WindowDict] = []
self._panes: list[PaneDict] = []

Expand Down Expand Up @@ -342,6 +344,7 @@ def cmd(
cmd: str,
*args: t.Any,
target: str | int | None = None,
timeout: float | None = None,
) -> tmux_cmd:
"""Execute tmux command respective of socket name and file, return output.

Expand Down Expand Up @@ -408,7 +411,12 @@ def cmd(

cmd_args = ["-t", str(target), *args] if target is not None else [*args]

return tmux_cmd(*svr_args, *cmd_args, tmux_bin=self.tmux_bin)
return tmux_cmd(
*svr_args,
*cmd_args,
tmux_bin=self.tmux_bin,
timeout=self.timeout if timeout is None else timeout,
)

@property
def attached_sessions(self) -> list[Session]:
Expand Down
28 changes: 28 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@

from __future__ import annotations

import typing as t

import pytest

from libtmux.common import get_version, get_version_str

if t.TYPE_CHECKING:
import pathlib


@pytest.fixture(autouse=True)
def _clear_get_version_cache() -> None:
Expand All @@ -20,3 +25,26 @@ def _clear_get_version_cache() -> None:
"""
get_version.cache_clear()
get_version_str.cache_clear()


@pytest.fixture
def hanging_tmux(tmp_path: pathlib.Path) -> tuple[str, pathlib.Path]:
"""Return a stand-in tmux that never answers, and its pid file.

Models the failure a real fixture cannot produce cheaply: a server
that accepts a connection and never replies. ``-V`` is answered,
because a wedged SERVER does not stop the local binary reporting its
own version, and `Server.sessions` reads it on the way past.
``exec`` preserves the recorded pid, so a test can check the process
afterwards instead of trusting that it was killed.
"""
pid_file = tmp_path / "pid"
binary = tmp_path / "tmux"
binary.write_text(
"#!/bin/sh\n"
'if [ "$1" = "-V" ]; then echo "tmux 3.7"; exit 0; fi\n'
f"echo $$ > {pid_file}\n"
"exec sleep 30\n"
)
binary.chmod(0o755)
return str(binary), pid_file
40 changes: 40 additions & 0 deletions tests/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

import locale
import logging
import os
import re
import sys
import time
import typing as t

import pytest
Expand All @@ -29,6 +31,8 @@
)

if t.TYPE_CHECKING:
import pathlib

from libtmux.server import Server
from libtmux.session import Session

Expand Down Expand Up @@ -762,3 +766,39 @@ def test_tmux_cmd_format_separator_survives_non_utf8_locale(
result = parse_output(line, "list-sessions", tmux_version)
assert isinstance(result, dict)
assert "session_id" in result


def test_tmux_cmd_timeout_kills_and_reaps(
hanging_tmux: tuple[str, pathlib.Path],
) -> None:
"""An expired command leaves no tmux process behind.

The kill is the load-bearing half. Without it a caller that gives up
only stops waiting, so repeated timeouts accumulate tmux processes
nobody is listening to.
"""
binary, pid_file = hanging_tmux

with pytest.raises(exc.TmuxTimeout) as excinfo:
tmux_cmd("list-sessions", tmux_bin=binary, timeout=0.3)

assert excinfo.value.timeout == 0.3
assert "list-sessions" in str(excinfo.value)

pid = int(pid_file.read_text())
with pytest.raises(ProcessLookupError):
os.kill(pid, 0)


def test_tmux_cmd_without_timeout_still_waits(
hanging_tmux: tuple[str, pathlib.Path],
) -> None:
"""The bound is opt-in; omitting it keeps the historical behaviour."""
binary, _pid_file = hanging_tmux
started = time.monotonic()

with pytest.raises(exc.TmuxTimeout):
tmux_cmd("list-sessions", tmux_bin=binary, timeout=0.3)

# A test that never waits would pass whether or not `timeout` is read.
assert time.monotonic() - started >= 0.3
31 changes: 31 additions & 0 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1765,3 +1765,34 @@ def test_server_display_message_warns_on_tmux_error(
"""
with pytest.warns(UserWarning, match="only one of -F or argument"):
server.display_message("x", get_text=True, format_string="#{version}")


def test_server_timeout_bounds_every_command(
hanging_tmux: tuple[str, pathlib.Path],
) -> None:
"""A server-level timeout is the policy for commands through it."""
binary, _pid_file = hanging_tmux
server = Server(tmux_bin=binary, timeout=0.3)

with pytest.raises(exc.TmuxTimeout):
server.cmd("list-sessions")


@pytest.mark.parametrize("accessor", ["sessions", "windows", "panes", "clients"])
def test_a_wedged_server_is_not_reported_as_empty(
hanging_tmux: tuple[str, pathlib.Path],
accessor: str,
) -> None:
"""A listing must raise rather than answer empty.

A tmux server that stopped answering has not said it has nothing.
Returning ``[]`` sends a caller on to create a session on a server
that already has them. Every accessor is covered because three of
them reimplemented the "empty means not ready" rule inline instead
of sharing it.
"""
binary, _pid_file = hanging_tmux
server = Server(tmux_bin=binary, timeout=0.3)

with pytest.raises(exc.TmuxTimeout):
_ = getattr(server, accessor)
Loading