From 1643c84aa79173e9a5da5dd33355613c566645bf Mon Sep 17 00:00:00 2001 From: Valerii Kovalskii Date: Thu, 13 Aug 2026 10:39:37 +0300 Subject: [PATCH] perf: stop the 5s active-sessions poll from blocking the event loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getActiveSessions() backs GET /api/active, which the dashboard polls every 5s. For Qwen agents it called findQwenSessionByPid(), which ran execSync('lsof -a -p -Fn') — with a 2000ms timeout, once per pid — inline. The server, the HTTP API and the browser-terminal WebSocket share one process and one event loop, so that stalled the pty data pump: typing in the terminal froze for up to two seconds per running agent, every five seconds. (7.15.0 moved the other ps/lsof polls off the loop; this one was missed.) lsof is kept — it is the only signal that can tell two sessions in the same folder apart — but it now runs through _execFileAsync off the loop and callers read a 30s per-pid cache, falling back to the existing cwd match until the first refresh lands. Refreshes are de-duplicated per pid and the cache is bounded. Also switches to argv form, so the pid is no longer interpolated into a shell command string. Note the measurement trap: on a machine with no running Qwen agent the hot loop never executes, so timing this looks green either way. The added test asserts the property structurally instead, and was verified to fail when execSync is reintroduced. Co-Authored-By: Claude Opus 5 (1M context) --- src/data.js | 53 +++++++++++++++----- test/active-poll-nonblocking.test.js | 74 ++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 11 deletions(-) create mode 100644 test/active-poll-nonblocking.test.js diff --git a/src/data.js b/src/data.js index a74166a..aa901e0 100644 --- a/src/data.js +++ b/src/data.js @@ -5949,24 +5949,55 @@ function findPiSessionByResumeTarget(resumeTarget, allSessions) { return (allSessions || []).find(s => s.tool === 'pi' && s.resume_target && path.resolve(s.resume_target) === resolvedTarget) || null; } +// Which Qwen transcript a pid currently has open, keyed by pid. +// `lsof` is the only precise signal (cwd-matching can't tell two sessions in the +// same folder apart), but it used to run through execSync RIGHT ON the +// /api/active path — which the dashboard polls every 5s. With a 2000ms timeout, +// per pid, that stalled the whole event loop, and the terminal WebSocket data +// pump with it: typing froze while the poll ran. So the lookup is now a cache +// refreshed OFF the loop; callers never wait for lsof. +const _qwenOpenFileCache = new Map(); // pid -> { ts, ids: string[] } +const _qwenOpenFileInflight = new Set(); // pids with a refresh in progress +const QWEN_LSOF_TTL = 30000; // an agent keeps its transcript open + +function _refreshQwenOpenFiles(pid) { + if (_qwenOpenFileInflight.has(pid)) return; + _qwenOpenFileInflight.add(pid); + _execFileAsync('lsof', ['-a', '-p', String(pid), '-Fn'], { timeout: 2000, maxBuffer: 8 * 1024 * 1024 }) + .then(({ stdout }) => { + const ids = []; + for (const line of String(stdout || '').split('\n')) { + const m = line.match(/(\/.*\.qwen\/projects\/.*\/(?:chats|sessions)\/([0-9a-f-]{36})\.jsonl)$/i); + if (m) ids.push(m[2]); + } + _qwenOpenFileCache.set(pid, { ts: Date.now(), ids }); + }) + .catch(() => { _qwenOpenFileCache.set(pid, { ts: Date.now(), ids: [] }); }) + .finally(() => { + _qwenOpenFileInflight.delete(pid); + // Bound the map: pids are recycled and agents come and go. + if (_qwenOpenFileCache.size > 256) { + for (const [k, v] of _qwenOpenFileCache) { + if (Date.now() - v.ts > QWEN_LSOF_TTL) _qwenOpenFileCache.delete(k); + } + } + }); +} + function findQwenSessionByPid(pid, cwd, allSessions) { const byOpenFile = []; const byCwd = []; - try { - const lsofOut = execSync(`lsof -a -p ${pid} -Fn 2>/dev/null`, { - encoding: 'utf8', - timeout: 2000, - stdio: ['pipe', 'pipe', 'pipe'], - }); - for (const line of lsofOut.split('\n')) { - const match = line.match(/(\/.*\.qwen\/projects\/.*\/(?:chats|sessions)\/([0-9a-f-]{36})\.jsonl)$/i); - if (!match) continue; - const sessionId = match[2]; + const cached = _qwenOpenFileCache.get(pid); + if (!cached || (Date.now() - cached.ts) > QWEN_LSOF_TTL) _refreshQwenOpenFiles(pid); + // Until the first refresh lands we simply fall through to the cwd match below, + // which is the same answer lsof would give for a single session in a folder. + if (cached) { + for (const sessionId of cached.ids) { const session = allSessions.find(s => s.id === sessionId); if (session) byOpenFile.push(session); } - } catch {} + } if (cwd) { for (const session of allSessions) { diff --git a/test/active-poll-nonblocking.test.js b/test/active-poll-nonblocking.test.js new file mode 100644 index 0000000..3e33539 --- /dev/null +++ b/test/active-poll-nonblocking.test.js @@ -0,0 +1,74 @@ +// Guards the /api/active path (polled every 5s by the dashboard) against +// synchronous subprocess calls. +// +// Why this matters: the server, the HTTP API and the browser-terminal +// WebSocket all share ONE process and ONE event loop. Anything synchronous on a +// polled path stalls the pty data pump, so typing in the terminal freezes. +// `findQwenSessionByPid` used to run `execSync('lsof …')` — with a 2000ms +// timeout, per pid — right inside `getActiveSessions()`. lsof is still the only +// precise signal for which transcript a pid has open, so it is kept, but the +// lookup now reads a cache refreshed off the loop. +// +// A timing assertion would be useless here (a dev machine usually has zero +// running Qwen agents, so the hot loop never executes and any measurement comes +// back green regardless). Instead this asserts the property structurally. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const SRC = fs.readFileSync(path.join(__dirname, '..', 'src', 'data.js'), 'utf8'); + +// Exact function body by brace matching — slicing "up to the next `function`" +// is not reliable here (neighbours are declared as `const x = …`, so the slice +// ran on for several functions), and comments must be stripped or a mere +// *mention* of execSync in prose trips the assertions. +function sliceFn(name) { + const start = SRC.indexOf('function ' + name); + assert.notEqual(start, -1, name + ' not found in data.js'); + let i = SRC.indexOf('{', start); + assert.notEqual(i, -1, name + ' has no body'); + let depth = 0; + for (; i < SRC.length; i++) { + if (SRC[i] === '{') depth++; + else if (SRC[i] === '}' && --depth === 0) { i++; break; } + } + return SRC.slice(start, i) + .replace(/\/\*[\s\S]*?\*\//g, '') // block comments + .replace(/^[ \t]*\/\/.*$/gm, ''); // line comments +} + +test('findQwenSessionByPid never shells out synchronously', () => { + const body = sliceFn('findQwenSessionByPid'); + assert.equal(/execSync|execFileSync|spawnSync/.test(body), false, + 'no synchronous subprocess call may run on the 5s /api/active path'); +}); + +test('the lsof lookup runs off the event loop and is cached', () => { + const body = sliceFn('_refreshQwenOpenFiles'); + assert.match(body, /_execFileAsync\(/, 'lsof must be invoked asynchronously'); + assert.match(body, /lsof/, 'still uses lsof — it is the only precise signal'); + // argv form, not a shell string: the pid is interpolated into the command. + assert.match(body, /\[\s*'-a'\s*,\s*'-p'/, 'lsof args must be passed as argv'); + assert.match(SRC, /_qwenOpenFileCache\s*=\s*new Map\(\)/, 'results must be cached per pid'); +}); + +test('an in-flight lsof refresh is not started twice for the same pid', () => { + const body = sliceFn('_refreshQwenOpenFiles'); + assert.match(body, /_qwenOpenFileInflight\.has\(pid\)/); + assert.match(body, /_qwenOpenFileInflight\.add\(pid\)/); + assert.match(body, /_qwenOpenFileInflight\.delete\(pid\)/); +}); + +test('the pid cache is bounded so it cannot grow without limit', () => { + const body = sliceFn('_refreshQwenOpenFiles'); + assert.match(body, /_qwenOpenFileCache\.size\s*>/, 'must cap the cache'); + assert.match(body, /_qwenOpenFileCache\.delete\(/, 'must evict stale entries'); +}); + +test('getActiveSessions itself stays free of synchronous subprocess calls', () => { + const body = sliceFn('getActiveSessions'); + assert.equal(/execSync\(|execFileSync\(|spawnSync\(/.test(body), false, + 'getActiveSessions is polled every 5s — it must not block the loop'); +});