mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 14:06:50 +01:00
fix(browse): read Xvfb's own startup error to classify a lost display race
Run 10 (1ffd38e) failed handoff's concurrent promotion once: 'Xvfb on :100
exited during startup (code 1)' with the display free again by the exit
check, so a holder that won the lock and closed within the 100 ms poll
looked like a broken install. Startup stderr now goes to a private file;
'Server is already active for display' is a collision and moves on, and any
other startup exit carries Xvfb's fatal line. Regression: a stub that
reports the collision for one display and exits leaves spawnFreeXvfb on the
next display (pre-fix: the misleading install error).
This commit is contained in:
1 parent
1ffd38eb46
commit
6290edd443
3 files changed
+61
-7
No files matched your search
+1
-1
@@ -81,7 +81,7 @@ The first two rows are the ones to care about: a review that read nothing, or fo
|
||||
- **Non-Claude hosts.** Later skill blocks lost `$GSTACK_*`, `$B` and `$D` and ran `/bin/gstack-...` (#1159). Codex skipped the symlinked router. design/dist and make-pdf/dist were missing from runtime roots (#2891). /ship and /plan-ceo-review overflowed the 160 KB host limit. The OpenCode `/gstack-review` command loaded nothing (#2651). `/browse` disappeared after toggling `skill_prefix` (#2263). Section files linked to checkout copies instead of rendered ones (#2707). skillify pointed at Claude's install on every host. /gstack-upgrade could run git cleanup in your own project when its path variable was unset.
|
||||
- **Design.** A design binary killed at launch reported `DESIGN_READY`. /design-consultation and /design-shotgun never loaded your taste profile. Plan reviews read a root DESIGN.md as the feature's design doc (#2840).
|
||||
- **/ship, /review and daily tools.** /ship stopped in versionless repos and monorepos (#2334, #2343). The plan audit picked an unrelated plan (#2797). Worktrees split project state (#2792). Learnings search exited 0 without Bun (#2794) and ranked poorly (#2796, #2799). Logs ignored `$TMPDIR` in sandboxed shells (#2952). /plan-tune said "calibrated" with no signals. /setup-deploy printed part of the Render API key (#1096). /analytics durations read 0s (#1728). Rejected taste preferences used the wrong confidence bucket (#1777). /context-save duration was empty on Linux (#2704). /retro missed pytest, minitest, XCTest, Terraform and Bats tests and counted build output (#2812, #2809, #2037). Plan reviews now treat design docs and handoff notes as data (#2818).
|
||||
- **Browser.** Browse failed behind `HTTP_PROXY` (#494). Headless browse and make-pdf ignored `GSTACK_CHROMIUM_PATH` (#2771, #1968, #1633); extensions opened a window (#2281). `prettyscreenshot --hide ... out.png` wrote elsewhere (#1419). Stopping the browse daemon took over 5 seconds because every Chromium close waited out its timeout; it now takes about 0.1 s, and a closed headed display no longer waits a second on its own exit. Two browse daemons going headed at the same moment could pick the same free X display, and the loser failed the handoff; it now takes the next free display. `gstack-ios-qa-daemon --help` started the daemon (#1932). Aside in `~/.local/bin` was not found (#2965).
|
||||
- **Browser.** Browse failed behind `HTTP_PROXY` (#494). Headless browse and make-pdf ignored `GSTACK_CHROMIUM_PATH` (#2771, #1968, #1633); extensions opened a window (#2281). `prettyscreenshot --hide ... out.png` wrote elsewhere (#1419). Stopping the browse daemon took over 5 seconds because every Chromium close waited out its timeout; it now takes about 0.1 s, and a closed headed display no longer waits a second on its own exit. Two browse daemons going headed at the same moment could pick the same free X display, and the loser failed the handoff; it now takes the next free display, and an Xvfb that fails to start reports its own error. `gstack-ios-qa-daemon --help` started the daemon (#1932). Aside in `~/.local/bin` was not found (#2965).
|
||||
- **Windows.** /freeze, /guard and /investigate denied every edit and their hooks failed under cmd.exe (#2876, #2354). Generators crashed with EEXIST (#2329). The node-server bundle inlined sharp and socks (#2260), and a minimal install needed `browse/src/server.ts` (#2439). The /cso helper failed on the current SDK and was reported as "install Visual Studio" (#3015). `gstack-config gbrain-refresh` misread status (#1981).
|
||||
- **Redaction.** Diagram and vector files flagged as phone and card numbers (#2885, #2827); function calls assigned to `session`/`token` (#2899); `<name>.local.<ext>` config filenames (#2962); four-part versions (#2784); `postgres:postgres@localhost` (#2913).
|
||||
- **Plan reviews.** /plan-eng-review writes its QA test plan even while some test choices are still pending, and its findings enter only through Scope Challenge C's saves. /qa-only's test value card no longer points at /qa's record step. /plan-ceo-review says what its closing cleanup does (old CEO plans are archived only with your approval).
|
||||
|
||||
+33
-6
@@ -188,12 +188,34 @@ export async function spawnXvfb(displayNum: number): Promise<XvfbHandle> {
|
||||
|
||||
// Spawn detached: Xvfb's lifetime is tied to whether we've explicitly
|
||||
// killed it via the handle's close() method, not to the parent process.
|
||||
const proc = Bun.spawn(['Xvfb', display, '-screen', '0', '1920x1080x24', '-ac'], {
|
||||
windowsHide: true,
|
||||
stdio: ['ignore', 'ignore', 'ignore'],
|
||||
});
|
||||
// Startup stderr goes to a private file: a holder that takes the display and
|
||||
// exits before we look leaves no lock behind, only Xvfb's own message.
|
||||
const stderrDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-xvfb-'));
|
||||
const stderrPath = path.join(stderrDir, 'stderr.log');
|
||||
const stderrFd = fs.openSync(stderrPath, 'w', 0o600);
|
||||
let proc: ReturnType<typeof Bun.spawn>;
|
||||
try {
|
||||
proc = Bun.spawn(['Xvfb', display, '-screen', '0', '1920x1080x24', '-ac'], {
|
||||
windowsHide: true,
|
||||
stdio: ['ignore', 'ignore', stderrFd],
|
||||
});
|
||||
} catch (err) {
|
||||
fs.rmSync(stderrDir, { recursive: true, force: true });
|
||||
throw err;
|
||||
} finally {
|
||||
fs.closeSync(stderrFd);
|
||||
}
|
||||
proc.unref();
|
||||
const startTime = readPidStartTime(proc.pid);
|
||||
try {
|
||||
return await awaitXvfbReady(proc, displayNum, startTime, stderrPath);
|
||||
} finally {
|
||||
fs.rmSync(stderrDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
async function awaitXvfbReady(proc: ReturnType<typeof Bun.spawn>, displayNum: number, startTime: string, stderrPath: string): Promise<XvfbHandle> {
|
||||
const display = `:${displayNum}`;
|
||||
|
||||
// Wait for the X server to become reachable — Xvfb takes a few hundred ms
|
||||
// to bind. Probe via xdpyinfo with retries.
|
||||
@@ -203,8 +225,13 @@ export async function spawnXvfb(displayNum: number): Promise<XvfbHandle> {
|
||||
await Bun.sleep(100);
|
||||
// If Xvfb crashed during startup, fail fast.
|
||||
if (proc.exitCode != null) {
|
||||
if (!isDisplayFree(displayNum)) throw new XvfbDisplayTakenError(`X display ${display} was reserved by another X server during startup`);
|
||||
throw new Error(`Xvfb on ${display} exited during startup (code ${proc.exitCode}). Hint: install xvfb (apt-get install xvfb / yum install xorg-x11-server-Xvfb).`);
|
||||
let stderr = '';
|
||||
try { stderr = fs.readFileSync(stderrPath, 'utf8'); } catch {}
|
||||
if (!isDisplayFree(displayNum) || /Server is already active for display/.test(stderr)) {
|
||||
throw new XvfbDisplayTakenError(`X display ${display} was reserved by another X server during startup`);
|
||||
}
|
||||
const fatal = stderr.split('\n').map(line => line.replace(/^\(EE\)\s*/, '').trim()).filter(Boolean).slice(-3).join(' ');
|
||||
throw new Error(`Xvfb on ${display} exited during startup (code ${proc.exitCode})${fatal ? `: ${fatal}` : ''}. Hint: install xvfb (apt-get install xvfb / yum install xorg-x11-server-Xvfb).`);
|
||||
}
|
||||
let ownsLock = false;
|
||||
try { ownsLock = Number(fs.readFileSync(`/tmp/.X${displayNum}-lock`, 'utf8').trim()) === proc.pid; } catch {}
|
||||
|
||||
@@ -261,6 +261,33 @@ describe.skipIf(process.platform !== 'linux')('display allocation failure contro
|
||||
}
|
||||
});
|
||||
|
||||
test.skipIf(!HAS_XVFB)('a display whose short-lived holder is gone by the exit check still counts as taken', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-xvfb-gone-'));
|
||||
const realXvfb = Bun.which('Xvfb')!;
|
||||
const first = pickFreeDisplay(26000, 26100);
|
||||
expect(first).not.toBeNull();
|
||||
// Run 10 (1ffd38e): another test's Xvfb held the display just long enough
|
||||
// to win the lock, then closed before the exit check looked for it.
|
||||
fs.writeFileSync(path.join(root, 'Xvfb'), [
|
||||
'#!/bin/sh',
|
||||
`if [ "$1" = ":${first}" ]; then printf '(EE) Server is already active for display ${first}\\n' >&2; exit 1; fi`,
|
||||
`exec ${JSON.stringify(realXvfb)} "$@"`, '',
|
||||
].join('\n'), { mode: 0o755 });
|
||||
try {
|
||||
const child = Bun.spawn([process.execPath, '-e', `
|
||||
import { spawnFreeXvfb } from ${JSON.stringify(path.resolve(import.meta.dir, '../src/xvfb.ts'))};
|
||||
try { const handle = await spawnFreeXvfb(${first}, ${first! + 3}); console.log(handle.display); handle.close(); }
|
||||
catch (err) { console.log(err.message); }
|
||||
`], { env: { ...process.env, PATH: `${root}:${process.env.PATH}` }, stdout: 'pipe', stderr: 'pipe', timeout: 20000 });
|
||||
const [stdout, exitCode] = await Promise.all([new Response(child.stdout).text(), child.exited]);
|
||||
expect(exitCode).toBe(0);
|
||||
expect(stdout.trim()).toBe(`:${first! + 1}`);
|
||||
expect(isDisplayFree(first!)).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
}, 30000);
|
||||
|
||||
test.skipIf(!HAS_XVFB)('concurrent allocators that pick the same display both get one', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-xvfb-race-'));
|
||||
const realXvfb = Bun.which('Xvfb')!;
|
||||
|
||||
Reference in new issue
Block a user