vnc: reclaim the port before starting, and do not trust a listener we did not spawn
Closes the TODO item about orphaned/duplicated VNC servers across sidecar restarts. The diagnosis there was correct and outlived the move off x11vnc, because the shape of the bug is in the lifecycle, not the server: the running desktop lives in module state, a sidecar restart forgets it while the process keeps running, and waitForPort accepted ANY listener on 5900 as proof of a healthy start. The next start would then spawn a server that could not bind the port, see the ORPHAN listening, and report success — leaving the platform convinced it had started a desktop the browser was not looking at. Two changes. reclaimPort frees the port before spawning: TERM whatever holds it, KILL after two seconds. waitForPort now also fails when the process we spawned has exited, so a foreign listener cannot be mistaken for our own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -136,10 +136,32 @@ async function isPortOpen(port: number): Promise<boolean> {
|
||||
}
|
||||
}
|
||||
|
||||
// Xvnc stays in the foreground, so readiness is the listening port rather than exit code.
|
||||
async function waitForPort(port: number): Promise<boolean> {
|
||||
// A listener on the port is NOT proof our server started. The running desktop is tracked in module
|
||||
// state, so a sidecar restart forgets it while the server keeps running orphaned — then the next start
|
||||
// spawns a second one, which cannot bind the port, while this check sees the ORPHAN listening and
|
||||
// reports success. The platform then believes it started a desktop the browser is not looking at.
|
||||
// So: reclaim the port before spawning, and treat our own process dying as failure.
|
||||
async function reclaimPort(port: number): Promise<void> {
|
||||
if (!(await isPortOpen(port))) return;
|
||||
|
||||
console.log(`[vnc] port ${port} already held — killing the orphan before starting`);
|
||||
Bun.spawnSync({ cmd: ['fuser', '-k', '-TERM', `${port}/tcp`], stdout: 'ignore', stderr: 'ignore' });
|
||||
|
||||
for (let i = 0; i < 20; i++) {
|
||||
await Bun.sleep(100);
|
||||
if (!(await isPortOpen(port))) return;
|
||||
}
|
||||
|
||||
Bun.spawnSync({ cmd: ['fuser', '-k', '-KILL', `${port}/tcp`], stdout: 'ignore', stderr: 'ignore' });
|
||||
await Bun.sleep(300);
|
||||
}
|
||||
|
||||
// Xvnc stays in the foreground, so readiness is the listening port rather than exit code — but only
|
||||
// once we know the process we spawned is the one still alive to own it.
|
||||
async function waitForPort(port: number, proc: { exitCode: number | null }): Promise<boolean> {
|
||||
const deadline = Date.now() + READY_TIMEOUT_MS;
|
||||
while (Date.now() < deadline) {
|
||||
if (proc.exitCode !== null) return false;
|
||||
if (await isPortOpen(port)) return true;
|
||||
await Bun.sleep(100);
|
||||
}
|
||||
@@ -155,6 +177,10 @@ export async function startSession(params: VncStartParams): Promise<{ port: numb
|
||||
const homeDir = getOwnerHomeDir(params.email);
|
||||
const { passwdFile } = await ensureVncPassword(homeDir);
|
||||
|
||||
// Before choosing a display: an orphan from a previous sidecar life would still hold the port, and
|
||||
// findFreeDisplay would then hand us a number whose server can never bind it.
|
||||
await reclaimPort(VNC_PORT);
|
||||
|
||||
const display = findFreeDisplay();
|
||||
const authFile = ensureXauthority(join(homeDir, '.vnc'), display);
|
||||
|
||||
@@ -203,13 +229,13 @@ export async function startSession(params: VncStartParams): Promise<{ port: numb
|
||||
}
|
||||
})();
|
||||
|
||||
if (!(await waitForPort(VNC_PORT))) {
|
||||
if (!(await waitForPort(VNC_PORT, proc))) {
|
||||
try {
|
||||
proc.kill();
|
||||
} catch {
|
||||
// already dead
|
||||
}
|
||||
throw new Error(`Xvnc failed to listen on ${VNC_PORT}: ${stderrTail.trim() || 'timed out'}`);
|
||||
throw new Error(`Xvnc failed to listen on ${VNC_PORT}: ${stderrTail.trim() || 'exited or timed out'}`);
|
||||
}
|
||||
|
||||
// The desktop itself, once the server is accepting. dbus-run-session gives this session its own bus:
|
||||
|
||||
Reference in New Issue
Block a user