fix(tmux): bound calls on our own server and run them with the server's own binary
A tmux 3.8 installed under a running 3.6 server left PATH's 3.8 client asking the 3.6 server, which spun at 100% CPU; the dashboard's synchronous `has-session` on it had no limit and froze every page, hook and terminal for five minutes (14:07–14:12 on the shared server). - Every call on our own server now has a 5 s limit and the member's one-minute back-off (`host: tmux server not answering` meanwhile), so a stuck server costs the event loop one timeout per minute, not one per poll. - Every call on a running server runs that server's own binary (`/proc/<pid>/exe`, found from its listening socket in /proc — asking tmux would be the very call that hangs), re-read every 10 s; PATH's tmux only starts a server or answers `-V`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -675,6 +675,16 @@ a pane really is. Each case below is refused or bounded, with a test in
|
|||||||
click makes on a member's pane (start, type, stop) still waits, up to the
|
click makes on a member's pane (start, type, stop) still waits, up to the
|
||||||
3 s. Every other synchronous call
|
3 s. Every other synchronous call
|
||||||
that runs as a member (git in their folder, `write-as`) stops after 5 s.
|
that runs as a member (git in their folder, `write-as`) stops after 5 s.
|
||||||
|
- **The host's own tmux server is bounded too.** Every call on it stops
|
||||||
|
after 5 s, and one that timed out is not asked again for a minute
|
||||||
|
(`host: tmux server not answering` meanwhile), so a spinning server can no
|
||||||
|
longer freeze the dashboard (it once did for five minutes). Every call on a
|
||||||
|
running host server uses that server's own binary (`/proc/<pid>/exe`, found
|
||||||
|
from its socket in `/proc`), so installing a newer tmux under a running
|
||||||
|
server no longer mixes versions: a 3.8 client asking a 3.6 server kept it at
|
||||||
|
100% CPU. A member's server is still asked with `TMUX_BIN` (the one path
|
||||||
|
sudoers allows), so after a tmux upgrade restart each member's tmux server
|
||||||
|
once it is idle (`sudo kill <pid>` of `tmux -S /srv/agenthub/users/<name>/.agenthub/tmux/sock`).
|
||||||
- **A member's huge or sparse files.** Transcripts and journals are read line
|
- **A member's huge or sparse files.** Transcripts and journals are read line
|
||||||
by line with a 64 MiB cap per line (a 512 MB line used to crash the server);
|
by line with a 64 MiB cap per line (a 512 MB line used to crash the server);
|
||||||
a whole member file is read only up to a cap (1 MiB for a login or a
|
a whole member file is read only up to a cap (1 MiB for a login or a
|
||||||
|
|||||||
@@ -344,3 +344,56 @@ describe("tmux wrapper", () => {
|
|||||||
assert.equal(tmux.upgradeAdvice("3.3.2", "freebsd"), null);
|
assert.equal(tmux.upgradeAdvice("3.3.2", "freebsd"), null);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("our own tmux server: bounded and version-matched", () => {
|
||||||
|
const fs = require("node:fs");
|
||||||
|
const os = require("node:os");
|
||||||
|
const path = require("node:path");
|
||||||
|
|
||||||
|
beforeEach(() => tmux.__reset());
|
||||||
|
|
||||||
|
it("backs off after a call to our server times out, instead of blocking every poll", () => {
|
||||||
|
let calls = 0;
|
||||||
|
tmux.__setExecImpl(() => {
|
||||||
|
calls++;
|
||||||
|
throw Object.assign(new Error("spawnSync tmux ETIMEDOUT"), { code: "ETIMEDOUT" });
|
||||||
|
});
|
||||||
|
assert.equal(tmux.hasSession("agenthub-lane-1"), false);
|
||||||
|
const after = calls;
|
||||||
|
assert.ok(after >= 1);
|
||||||
|
// Meanwhile nothing reaches tmux: the server is left alone for a minute.
|
||||||
|
assert.equal(tmux.hasSession("agenthub-lane-1"), false);
|
||||||
|
assert.equal(tmux.serverPid(), null);
|
||||||
|
assert.equal(calls, after);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("finds the binary of the server listening on our socket from /proc alone", () => {
|
||||||
|
const root = fs.mkdtempSync(path.join(os.tmpdir(), "tmux-proc-"));
|
||||||
|
const sock = "/tmp/tmux-1000/default";
|
||||||
|
fs.mkdirSync(path.join(root, "net"));
|
||||||
|
fs.writeFileSync(
|
||||||
|
path.join(root, "net", "unix"),
|
||||||
|
"Num RefCount Protocol Flags Type St Inode Path\n" +
|
||||||
|
`0000: 00000003 00000000 00000000 0001 03 777 ${sock}\n` + // a client's
|
||||||
|
`0000: 00000002 00000000 00010000 0001 01 4242 ${sock}\n` // the listener
|
||||||
|
);
|
||||||
|
const proc = (pid, comm, fds) => {
|
||||||
|
fs.mkdirSync(path.join(root, pid, "fd"), { recursive: true });
|
||||||
|
fs.writeFileSync(path.join(root, pid, "comm"), `${comm}\n`);
|
||||||
|
fs.symlinkSync(process.execPath, path.join(root, pid, "exe"));
|
||||||
|
fds.forEach((t, i) => fs.symlinkSync(t, path.join(root, pid, "fd", String(i))));
|
||||||
|
};
|
||||||
|
proc("10", "bash", ["socket:[4242]"]); // not tmux
|
||||||
|
proc("11", "tmux: client", ["socket:[777]"]);
|
||||||
|
proc("12", "tmux: server", ["/dev/null", "socket:[4242]"]);
|
||||||
|
try {
|
||||||
|
assert.equal(
|
||||||
|
tmux.serverExeFor(sock, { procRoot: root, uid: null }),
|
||||||
|
path.join(root, "12", "exe")
|
||||||
|
);
|
||||||
|
assert.equal(tmux.serverExeFor("/tmp/other", { procRoot: root, uid: null }), null);
|
||||||
|
} finally {
|
||||||
|
fs.rmSync(root, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
+116
-11
@@ -27,10 +27,16 @@
|
|||||||
* a member's runs in the background and a poll reads its last answer.
|
* a member's runs in the background and a poll reads its last answer.
|
||||||
* With no member (local mode, S3 not installed, the host's own workspaces)
|
* With no member (local mode, S3 not installed, the host's own workspaces)
|
||||||
* nothing changes.
|
* nothing changes.
|
||||||
|
* Calls on our OWN server are bounded like a member's (5 s, then a minute's
|
||||||
|
* back-off) and run that server's own binary (`/proc/<pid>/exe`, found from
|
||||||
|
* its socket), so a tmux upgraded under it never mixes client and server
|
||||||
|
* versions — a 3.8 client spun a 3.6 server at 100% CPU.
|
||||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||||
*/
|
*/
|
||||||
|
|
||||||
const { execFile, execFileSync } = require("node:child_process");
|
const { execFile, execFileSync } = require("node:child_process");
|
||||||
|
const fs = require("node:fs");
|
||||||
|
const path = require("node:path");
|
||||||
|
|
||||||
// Test seam: swap the exec implementation so unit tests never invoke a real
|
// Test seam: swap the exec implementation so unit tests never invoke a real
|
||||||
// tmux binary. Mirrors run-spawner.js's __injectChildForTest/__reset style.
|
// tmux binary. Mirrors run-spawner.js's __injectChildForTest/__reset style.
|
||||||
@@ -40,8 +46,88 @@ const { execFile, execFileSync } = require("node:child_process");
|
|||||||
// journal on every poll while no tmux server is running yet.
|
// journal on every poll while no tmux server is running yet.
|
||||||
// cwd "/": a tmux server is forked from the call that starts it and keeps
|
// cwd "/": a tmux server is forked from the call that starts it and keeps
|
||||||
// that cwd; once it is deleted, tmux ignores every pane's `-c` (issue #69).
|
// that cwd; once it is deleted, tmux ignores every pane's `-c` (issue #69).
|
||||||
const EXEC_OPTS = { encoding: "utf8", stdio: ["pipe", "pipe", "ignore"], cwd: "/" };
|
// Bounded like a member's call (below): our server can spin too — a 3.6
|
||||||
let execImpl = (args, opts = EXEC_OPTS) => execFileSync("tmux", args, opts);
|
// server asked by a 3.8 client sat at 100% CPU, and one unbounded
|
||||||
|
// `has-session` froze the whole dashboard for five minutes.
|
||||||
|
const HOST_TIMEOUT_MS = 5000;
|
||||||
|
const EXEC_OPTS = {
|
||||||
|
encoding: "utf8",
|
||||||
|
stdio: ["pipe", "pipe", "ignore"],
|
||||||
|
cwd: "/",
|
||||||
|
timeout: HOST_TIMEOUT_MS,
|
||||||
|
killSignal: "SIGKILL",
|
||||||
|
maxBuffer: 1 << 20,
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The binary that started the tmux server listening on `socket`
|
||||||
|
* (`/proc/<pid>/exe`, which still runs after an upgrade replaced the file),
|
||||||
|
* or null. A client must match its server: tmux upgraded under a running
|
||||||
|
* server left PATH's newer client talking to the old server, which spun at
|
||||||
|
* 100% CPU on it. Found from /proc alone — asking tmux would BE that call.
|
||||||
|
* `procRoot`/`uid` are test seams.
|
||||||
|
*/
|
||||||
|
function serverExeFor(socket, { procRoot = "/proc", uid = process.getuid?.() } = {}) {
|
||||||
|
let inode = null;
|
||||||
|
try {
|
||||||
|
for (const line of fs.readFileSync(path.join(procRoot, "net", "unix"), "utf8").split("\n")) {
|
||||||
|
// Num RefCount Protocol Flags Type St Inode Path — Flags 00010000 = listening.
|
||||||
|
const f = line.trim().split(/\s+/);
|
||||||
|
if (f.length >= 8 && f[3] === "00010000" && f.slice(7).join(" ") === socket) {
|
||||||
|
inode = f[6];
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
if (!inode) return null;
|
||||||
|
const want = `socket:[${inode}]`;
|
||||||
|
for (const pid of fs.readdirSync(procRoot)) {
|
||||||
|
if (!/^\d+$/.test(pid)) continue;
|
||||||
|
const dir = path.join(procRoot, pid);
|
||||||
|
try {
|
||||||
|
if (!fs.readFileSync(path.join(dir, "comm"), "utf8").startsWith("tmux")) continue;
|
||||||
|
if (uid != null && fs.statSync(dir).uid !== uid) continue;
|
||||||
|
for (const fd of fs.readdirSync(path.join(dir, "fd"))) {
|
||||||
|
if (fs.readlinkSync(path.join(dir, "fd", fd)) !== want) continue;
|
||||||
|
const exe = path.join(dir, "exe");
|
||||||
|
fs.accessSync(exe, fs.constants.X_OK);
|
||||||
|
return exe;
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
/* gone, or not ours to read */
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Our default socket, as tmux itself picks it (no -L/-S). */
|
||||||
|
function hostSocket() {
|
||||||
|
if (process.env.TMUX) return process.env.TMUX.split(",")[0];
|
||||||
|
const dir = process.env.TMUX_TMPDIR || "/tmp";
|
||||||
|
return path.join(dir, `tmux-${process.getuid()}`, "default");
|
||||||
|
}
|
||||||
|
|
||||||
|
// The binary every call on OUR server runs, re-read every HOST_BIN_TTL_MS.
|
||||||
|
// PATH's tmux when no server runs (that call starts one), off Linux, and for
|
||||||
|
// `-V` (the installed client's version, for the upgrade advice).
|
||||||
|
const HOST_BIN_TTL_MS = 10_000;
|
||||||
|
let hostBin = { at: 0, bin: "tmux" };
|
||||||
|
function hostTmuxBin() {
|
||||||
|
if (process.platform !== "linux") return "tmux";
|
||||||
|
if (Date.now() - hostBin.at < HOST_BIN_TTL_MS) return hostBin.bin;
|
||||||
|
let bin = "tmux";
|
||||||
|
try {
|
||||||
|
bin = serverExeFor(hostSocket()) || "tmux";
|
||||||
|
} catch {
|
||||||
|
/* no /proc: PATH's */
|
||||||
|
}
|
||||||
|
hostBin = { at: Date.now(), bin };
|
||||||
|
return bin;
|
||||||
|
}
|
||||||
|
let execImpl = (args, opts = EXEC_OPTS) =>
|
||||||
|
execFileSync(args[0] === "-V" ? "tmux" : hostTmuxBin(), args, opts);
|
||||||
|
|
||||||
// Linux: the tmux server lives in the cgroup of whatever started it — the
|
// Linux: the tmux server lives in the cgroup of whatever started it — the
|
||||||
// dashboard's systemd unit, or the terminal/IDE scope a dev server runs in —
|
// dashboard's systemd unit, or the terminal/IDE scope a dev server runs in —
|
||||||
@@ -136,6 +222,21 @@ function memberCall(acct, args, input) {
|
|||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Our own server under the same back-off: a call that timed out stops every
|
||||||
|
// call on it for MEMBER_BACKOFF_MS (ETMUXHUNG), so a spinning server costs
|
||||||
|
// the event loop one HOST_TIMEOUT_MS per minute, not one per poll.
|
||||||
|
const HOST_ACCT = { login: "host" };
|
||||||
|
function hostCall(args) {
|
||||||
|
reachable(HOST_ACCT);
|
||||||
|
try {
|
||||||
|
return execImpl(args);
|
||||||
|
} catch (err) {
|
||||||
|
noteHang(HOST_ACCT, err);
|
||||||
|
if (err.code === "ETIMEDOUT") hostBin.at = 0; // look for the server again
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/** Test seam. A sync `member` fake also answers the async listing. */
|
/** Test seam. A sync `member` fake also answers the async listing. */
|
||||||
function __setAccountImpl(fn, member) {
|
function __setAccountImpl(fn, member) {
|
||||||
accountImpl = fn;
|
accountImpl = fn;
|
||||||
@@ -159,7 +260,7 @@ function accountOf(name) {
|
|||||||
/** Run a tmux command on the server that holds session `name`. */
|
/** Run a tmux command on the server that holds session `name`. */
|
||||||
function onServerOf(name, args, input) {
|
function onServerOf(name, args, input) {
|
||||||
const acct = accountOf(name);
|
const acct = accountOf(name);
|
||||||
return acct ? memberCall(acct, args, input) : execImpl(args);
|
return acct ? memberCall(acct, args, input) : hostCall(args);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Whether this tmux takes `=name` targets: true for tmux itself, false for
|
// Whether this tmux takes `=name` targets: true for tmux itself, false for
|
||||||
@@ -174,12 +275,15 @@ function __setExecImpl(fn, { exact = true } = {}) {
|
|||||||
exactTargets = exact;
|
exactTargets = exact;
|
||||||
listed = new Map();
|
listed = new Map();
|
||||||
listGen++;
|
listGen++;
|
||||||
|
unreachable = new Map();
|
||||||
}
|
}
|
||||||
function __setScopeImpl(fn) {
|
function __setScopeImpl(fn) {
|
||||||
scopeImpl = fn;
|
scopeImpl = fn;
|
||||||
}
|
}
|
||||||
function __reset() {
|
function __reset() {
|
||||||
execImpl = (args, opts = EXEC_OPTS) => execFileSync("tmux", args, opts);
|
execImpl = (args, opts = EXEC_OPTS) =>
|
||||||
|
execFileSync(args[0] === "-V" ? "tmux" : hostTmuxBin(), args, opts);
|
||||||
|
hostBin = { at: 0, bin: "tmux" };
|
||||||
scopeImpl = realScopeImpl;
|
scopeImpl = realScopeImpl;
|
||||||
accountImpl = (name) => require("./run-as").accountForSession(name);
|
accountImpl = (name) => require("./run-as").accountForSession(name);
|
||||||
memberImpl = realMemberImpl;
|
memberImpl = realMemberImpl;
|
||||||
@@ -217,7 +321,7 @@ function listOutput(acct = null) {
|
|||||||
return hit.out;
|
return hit.out;
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
const out = sessionNames(execImpl(LIST_ARGS));
|
const out = sessionNames(hostCall(LIST_ARGS));
|
||||||
listed.set("", { at: Date.now(), out });
|
listed.set("", { at: Date.now(), out });
|
||||||
return out;
|
return out;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
@@ -324,7 +428,7 @@ function exact() {
|
|||||||
if (exactTargets !== null) return exactTargets;
|
if (exactTargets !== null) return exactTargets;
|
||||||
let first;
|
let first;
|
||||||
try {
|
try {
|
||||||
first = sessionNames(execImpl(LIST_ARGS))
|
first = sessionNames(hostCall(LIST_ARGS))
|
||||||
.split("\n")
|
.split("\n")
|
||||||
.map((x) => x.trim())
|
.map((x) => x.trim())
|
||||||
.find(Boolean);
|
.find(Boolean);
|
||||||
@@ -334,7 +438,7 @@ function exact() {
|
|||||||
if (!first) return true;
|
if (!first) return true;
|
||||||
const answers = (t) => {
|
const answers = (t) => {
|
||||||
try {
|
try {
|
||||||
execImpl(["has-session", "-t", t]);
|
hostCall(["has-session", "-t", t]);
|
||||||
return true;
|
return true;
|
||||||
} catch {
|
} catch {
|
||||||
return false;
|
return false;
|
||||||
@@ -379,7 +483,7 @@ function hasSession(name) {
|
|||||||
/** Our running tmux server's pid, or null when none answers. */
|
/** Our running tmux server's pid, or null when none answers. */
|
||||||
function serverPid() {
|
function serverPid() {
|
||||||
try {
|
try {
|
||||||
const pid = String(execImpl(["display-message", "-p", "#{pid}"])).trim();
|
const pid = String(hostCall(["display-message", "-p", "#{pid}"])).trim();
|
||||||
return /^\d+$/.test(pid) ? pid : null;
|
return /^\d+$/.test(pid) ? pid : null;
|
||||||
} catch {
|
} catch {
|
||||||
return null;
|
return null;
|
||||||
@@ -435,10 +539,10 @@ function newSession({ name, cwd, argv, env }) {
|
|||||||
scopeImpl(args);
|
scopeImpl(args);
|
||||||
} catch {
|
} catch {
|
||||||
// No user manager (SSH without linger, no systemd): the plain start.
|
// No user manager (SSH without linger, no systemd): the plain start.
|
||||||
if (!hasSession(name)) execImpl(args);
|
if (!hasSession(name)) hostCall(args);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
execImpl(args);
|
hostCall(args);
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
enableMouse(name);
|
enableMouse(name);
|
||||||
@@ -733,7 +837,7 @@ function upgradeAdvice(version = clientVersion(), platform = process.platform) {
|
|||||||
* an upgrade keeps its old version until every tmux session has closed. */
|
* an upgrade keeps its old version until every tmux session has closed. */
|
||||||
function serverVersion() {
|
function serverVersion() {
|
||||||
try {
|
try {
|
||||||
return execImpl(["display-message", "-p", "#{version}"]).trim() || null;
|
return hostCall(["display-message", "-p", "#{version}"]).trim() || null;
|
||||||
} catch {
|
} catch {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
@@ -741,6 +845,7 @@ function serverVersion() {
|
|||||||
|
|
||||||
module.exports = {
|
module.exports = {
|
||||||
__setScopeImpl,
|
__setScopeImpl,
|
||||||
|
serverExeFor,
|
||||||
session,
|
session,
|
||||||
pane,
|
pane,
|
||||||
hasSession,
|
hasSession,
|
||||||
|
|||||||
Reference in New Issue
Block a user