Skip to content

Commit b842274

Browse files
committed
ControlMode(fix[control_mode]): replace FIFO with os.pipe(), add cleanup, socket_path
why: tempfile.mktemp() has a TOCTOU race; failure-path left open fds and live subprocess; socket_path was silently ignored. what: - Replace tempfile.mktemp() + os.mkfifo() FIFO with os.pipe() pair (no filesystem, no race condition) - Add failure-path cleanup in __enter__: if retry_until fails, close _write_fd and terminate subprocess before re-raising - Handle socket_path: build socket_args checking socket_name first (-L), then socket_path (-S), then empty - Remove unused tempfile and pathlib imports
1 parent f336a59 commit b842274

File tree

1 file changed

+36
-29
lines changed

1 file changed

+36
-29
lines changed

src/libtmux/_internal/control_mode.py

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88
from __future__ import annotations
99

1010
import os
11-
import pathlib
1211
import subprocess
13-
import tempfile
1412
import typing as t
1513

1614
from libtmux.test.retry import retry_until
@@ -52,7 +50,6 @@ class ControlMode:
5250
stdout: t.IO[str]
5351

5452
_proc: subprocess.Popen[str]
55-
_fifo_path: str
5653
_write_fd: int
5754

5855
def __init__(self, server: Server, session: Session) -> None:
@@ -61,34 +58,38 @@ def __init__(self, server: Server, session: Session) -> None:
6158

6259
def __enter__(self) -> ControlMode:
6360
"""Spawn control-mode client and wait for registration."""
64-
self._fifo_path = tempfile.mktemp(prefix="libtmux_ctl_")
65-
os.mkfifo(self._fifo_path)
61+
read_fd, self._write_fd = os.pipe()
6662

6763
tmux_bin = self.server.tmux_bin or "tmux"
64+
65+
# Build socket arguments matching Server's own logic
66+
if self.server.socket_name is not None:
67+
socket_args = ["-L", str(self.server.socket_name)]
68+
elif self.server.socket_path is not None:
69+
socket_args = ["-S", str(self.server.socket_path)]
70+
else:
71+
socket_args = []
72+
6873
cmd = [
6974
tmux_bin,
70-
"-L",
71-
str(self.server.socket_name),
75+
*socket_args,
7276
"-C",
7377
"attach-session",
7478
"-t",
7579
str(self.session.session_id),
7680
]
7781

78-
# Open read end for subprocess stdin
79-
read_fd = os.open(self._fifo_path, os.O_RDONLY | os.O_NONBLOCK)
80-
81-
self._proc = subprocess.Popen(
82-
cmd,
83-
stdin=read_fd,
84-
stdout=subprocess.PIPE,
85-
stderr=subprocess.PIPE,
86-
text=True,
87-
)
88-
os.close(read_fd)
89-
90-
# Open write end to keep FIFO alive
91-
self._write_fd = os.open(self._fifo_path, os.O_WRONLY)
82+
try:
83+
self._proc = subprocess.Popen(
84+
cmd,
85+
stdin=read_fd,
86+
stdout=subprocess.PIPE,
87+
stderr=subprocess.PIPE,
88+
text=True,
89+
)
90+
finally:
91+
# Close read end in parent regardless — subprocess owns it now
92+
os.close(read_fd)
9293

9394
self.stdout = self._proc.stdout # type: ignore[assignment]
9495
client_pid = str(self._proc.pid)
@@ -107,7 +108,18 @@ def client_registered() -> bool:
107108
return True
108109
return False
109110

110-
retry_until(client_registered, 3, raises=True)
111+
try:
112+
retry_until(client_registered, 3, raises=True)
113+
except Exception:
114+
# Clean up subprocess and write end if registration fails
115+
os.close(self._write_fd)
116+
self._proc.terminate()
117+
try:
118+
self._proc.wait(timeout=5)
119+
except subprocess.TimeoutExpired:
120+
self._proc.kill()
121+
self._proc.wait()
122+
raise
111123

112124
return self
113125

@@ -117,8 +129,8 @@ def __exit__(
117129
exc_val: BaseException | None,
118130
exc_tb: types.TracebackType | None,
119131
) -> None:
120-
"""Terminate control-mode client and clean up FIFO."""
121-
# Close write end — causes the control-mode client to exit
132+
"""Terminate control-mode client."""
133+
# Close write end — causes the control-mode client to exit (EOF on stdin)
122134
os.close(self._write_fd)
123135

124136
# Terminate and wait for subprocess
@@ -128,8 +140,3 @@ def __exit__(
128140
except subprocess.TimeoutExpired:
129141
self._proc.kill()
130142
self._proc.wait()
131-
132-
# Remove FIFO
133-
fifo = pathlib.Path(self._fifo_path)
134-
if fifo.exists():
135-
fifo.unlink()

0 commit comments

Comments
 (0)