Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/specs/vscode.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ Source of truth: `VsCodeBurrowStateStore` in `vscode-ext/src/burrow-store.ts`, `

- **Roles never flip downward.** There is no `onRole(false)` after a `true`, and a client only ever changes role *upward*, which makes a TTL lease's mid-transition races unrepresentable rather than handled (rationale).
- **Contend on broker death, not on a timer.** When the broker exits, every client's socket closes and they all race to bind; exactly one wins, because `bind` is the arbiter. No TTL, no heartbeat file, no filesystem watcher.
- **A corpse is cleared, then the bind is re-checked.** **Never unlink on the first refusal**, and **`stillOurs` compares full filesystem identity — device, inode, and nanosecond change timestamp, never inode alone**. A window whose socket identity was replaced, **or whose path has gone entirely**, stands down and the loop re-runs (jitter, re-dial and per-platform reasoning at `attempt` and `stillOurs`; rationale).
- **A corpse is cleared, then the bind is re-checked.** **Never unlink on the first refusal**, and **`stillOurs` compares full filesystem identity — device, inode, and nanosecond change timestamp, never inode alone, anchored at our own bind rather than a later read of the path**. A window whose socket identity was replaced, **or whose path has gone entirely**, stands down and the loop re-runs (jitter, re-dial and per-platform reasoning at `attempt` and `stillOurs`; rationale).
- **A bind is not a role until it is believed.** Everything that answers "is this window the broker" — `ensurePeerNet`'s shortcut, `isPeerBroker`, `isPeerLinkSettled`, `remoteNotifyPeerChange` — reads `brokerConfirmed`, set only where `settle(true)` runs and cleared by `closeServer`. **Unverified reads as unsettled**, so a command landing in the `RECLAIM_VERIFY_MS` window (an `enroll`, a `secrets.onDidChange`) is held for the verdict rather than told "broker" (rationale).
- **Attempts are spaced.** The loop waits `RETRY_MS` between rounds so a refused hello cannot spin, and a bind or connect landing after disposal is undone rather than left to outlive its window.
- **Errors after `listen` are logged, not thrown** — an `EventEmitter` with no `'error'` listener rethrows out of a libuv callback and takes the extension host down, so `listenServer` installs a permanent logging listener the moment the bind succeeds (rationale).
Expand Down
2 changes: 1 addition & 1 deletion docs/specs/vscode.rationale.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ Rows 1–2 are why a blanket second press is wrong; `Press Ctrl-C again` was abs

**Why the reclaim is jittered and re-dialled.** Every client of a dead broker reaches the `ECONNREFUSED` at the same instant. Unlinking immediately means several of them unlink, and unlinking a *live* broker's socket — one that rebound the path while we waited — strands every window dialling it.

**Why `stillOurs` compares full filesystem identity.** Two windows can find the same corpse, both unlink, and the second bind silently displaces the first, leaving the loser serving a socket no client can reach; nothing on the bind path detects that. Inode alone is reused too readily to distinguish "still ours" from "replaced".
**Why `stillOurs` compares full filesystem identity.** Two windows can find the same corpse, both unlink, and the second bind silently displaces the first, leaving the loser serving a socket no client can reach; nothing on the bind path detects that. Inode alone is reused too readily to distinguish "still ours" from "replaced". And the identity has to be anchored to our own bind rather than to a first read of the path: the displacing unlink and rebind can land before that read, so both windows read the winner's socket, both find it unchanged, and both confirm — two brokers, which is what `settles two windows racing for one corpse into a broker and a client` caught intermittently (observed 2026-09-22). `listen` binds inside the call and resolves on a nextTick, so a `statSync` before the next `await` runs ahead of anything queued on this process's thread pool. The competitor is another extension host, so its unlink and rebind can still land in the microseconds between our `bind` and that `statSync`: the anchor narrows the race rather than closing it. Closing it would take an atomic create-only publish — bind a unique path, then `link()` it onto the fixed one, which fails `EEXIST` instead of displacing — a redesign of the arbitration.

**What an unverified bind would cost.** During `RECLAIM_VERIFY_MS` the socket is bound but may still be given up. A command landing inside that window and told "broker" would start a service the stand-down path never tears down — two Burrows under one burrowId, and the endless relay displacement above.

Expand Down
2 changes: 1 addition & 1 deletion scripts/spec-word-budgets.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"docs/specs/tiling-engine.md": 4650,
"docs/specs/transport.md": 5500,
"docs/specs/tutorial.md": 1900,
"docs/specs/vscode.md": 7200,
"docs/specs/vscode.md": 7250,
"docs/specs/webgl-text.md": 1200,
"docs/specs/website-docs.md": 4800
}
43 changes: 38 additions & 5 deletions vscode-ext/src/peer-link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { chmod, lstat, mkdir, readFile, rm, stat, writeFile } from 'node:fs/promises';
import { statSync, type BigIntStats } from 'node:fs';
import { createHash, randomUUID } from 'node:crypto';
import { createConnection, createServer, type Server, type Socket } from 'node:net';
import { tmpdir } from 'node:os';
Expand Down Expand Up @@ -287,6 +288,18 @@ let server: Server | null = null;
let brokerConfirmed = false;
/** Claimed and cleared with `server`; the two always move together. */
let serverToken: string | null = null;
/**
* The socket file this window's own bind created, claimed and cleared with
* `server`.
*
* {@link stillOurs} compares the path against *this*, never against a second
* read of the path: a competing window that cleared the same corpse may have
* rebound it before an async read lands, and then both reads name that
* window's socket, the two agree, and every window that bound believes it
* won. Captured synchronously after the bind, which narrows that gap without
* closing it (see `tryBind`).
*/
let boundSocketFile: SocketFileIdentity | null = null;
const clients = new Set<PeerLinkClient>();
/** Provider-local route handle → the peer window that owns it. */
const routes = new Map<string, PeerLinkClient>();
Expand Down Expand Up @@ -783,6 +796,15 @@ async function tryBind(path: string, token: string): Promise<boolean> {
return false;
}
server = nextServer;
// Synchronous, and before the first `await` past the bind: `listen` binds
// inside the call and resolves on a nextTick, so no thread-pool callback of
// *this* process can interleave. A competing window is a separate extension
// host, so its unlink and rebind can still land here — this narrows the gap
// to a few microseconds of straight-line code rather than closing it, and
// reads the file our own bind made ({@link boundSocketFile}). Skipped on
// Windows: a named pipe is not a filesystem object, so there is no anchor
// to take and nothing there can displace us ({@link stillOurs}).
boundSocketFile = process.platform === 'win32' ? null : socketFileIdentitySync(path);
// Provisional until the caller settles it: a reclaimed bind may still be
// displaced (see {@link brokerConfirmed}).
brokerConfirmed = false;
Expand Down Expand Up @@ -1243,11 +1265,21 @@ interface SocketFileIdentity {
ctimeNs: bigint;
}

function toSocketFileIdentity(value: BigIntStats | null | undefined): SocketFileIdentity | null {
return value ? { dev: value.dev, ino: value.ino, ctimeNs: value.ctimeNs } : null;
}

async function socketFileIdentity(path: string): Promise<SocketFileIdentity | null> {
const value = await stat(path, { bigint: true }).catch(() => null);
return value
? { dev: value.dev, ino: value.ino, ctimeNs: value.ctimeNs }
: null;
return toSocketFileIdentity(await stat(path, { bigint: true }).catch(() => null));
}

/** {@link socketFileIdentity} without yielding — see {@link boundSocketFile}. */
function socketFileIdentitySync(path: string): SocketFileIdentity | null {
try {
return toSocketFileIdentity(statSync(path, { bigint: true, throwIfNoEntry: false }));
} catch {
return null;
}
}

function sameSocketFile(left: SocketFileIdentity, right: SocketFileIdentity): boolean {
Expand All @@ -1256,7 +1288,7 @@ function sameSocketFile(left: SocketFileIdentity, right: SocketFileIdentity): bo

async function stillOurs(path: string): Promise<boolean> {
const unstattable = process.platform === 'win32';
const mine = await socketFileIdentity(path);
const mine = boundSocketFile;
if (!mine) return unstattable;
await delay(RECLAIM_VERIFY_MS);
const now = await socketFileIdentity(path);
Expand Down Expand Up @@ -1296,6 +1328,7 @@ async function closeServer(unlink: boolean): Promise<void> {
server = null;
brokerConfirmed = false;
serverToken = null;
boundSocketFile = null;
for (const peer of [...clients]) dropClient(peer);
if (!closing) return;
if (closing.listening) closing.close();
Expand Down
69 changes: 69 additions & 0 deletions vscode-ext/test/peer-link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,26 @@ vi.mock('../src/log', () => ({
},
}));

/**
* Runs once, just before the next `stat` of the peer socket path — the only
* `stat` `peer-link` makes. Lets a test land a competing window's displacement
* at an exact point in the reclaim verification. Hoisted for the same reason as
* {@link logged}.
*/
const beforeSocketStat = vi.hoisted(() => ({ hook: null as null | (() => Promise<void>), path: '' }));
vi.mock('node:fs/promises', async (importOriginal) => {
const real = await importOriginal<typeof import('node:fs/promises')>();
const stat = (async (...args: Parameters<typeof real.stat>) => {
const { hook, path } = beforeSocketStat;
if (hook && String(args[0]) === path) {
beforeSocketStat.hook = null;
await hook();
}
return real.stat(...args);
}) as typeof real.stat;
return { ...real, stat };
});

let dir: string;
/** Peer sockets live in the temp dir; point that at this test's own storage. */
let realTmp: string | undefined;
Expand Down Expand Up @@ -133,6 +153,7 @@ beforeEach(async () => {
realTmp = process.env.TMPDIR;
process.env.TMPDIR = dir;
logged.length = 0;
beforeSocketStat.hook = null;
});

afterEach(async () => {
Expand Down Expand Up @@ -457,6 +478,54 @@ describe('bind-as-lease', () => {
expect(firstRoles.concat(secondRoles)).toEqual([true]);
}, 30_000);

it('stands down when a competing reclaim displaces it before its verification reads the path', async () => {
// The interleaving the racing test above reaches only by luck, forced: a
// competing window's unlink and rebind land after our bind but before
// `stillOurs` first reads the path. Anchored to that read rather than to
// our own bind, both windows would name the competitor's socket as "ours"
// and both would broker.
const path = derivedSocketPath();
await mkdir(dirname(path), { recursive: true, mode: 0o700 });
const corpse = spawn(process.execPath, [
'-e',
`require('node:net').createServer().listen(${JSON.stringify(path)})`,
]);
await waitForFile(path);
corpse.kill('SIGKILL');
await new Promise((resolve) => corpse.on('exit', resolve));

// The competitor holds the path and never speaks, standing in for a window
// that won the reclaim.
const held: Socket[] = [];
const competitor = createServer((socket) => void held.push(socket));
Comment thread
dormouse-bot marked this conversation as resolved.
let theirs: SocketFileIdentity | null = null;
beforeSocketStat.path = path;
beforeSocketStat.hook = async () => {
await rm(path, { force: true });
await new Promise<void>((resolve) => competitor.listen(path, resolve));
theirs = await socketFileIdentity(path);
};
try {
const mod = await openWindow(fakeWindow());
const roles: boolean[] = [];
await mod.ensurePeerNet((broker) => roles.push(broker));
expect(theirs).not.toBeNull();
// Settled as broker either way — standing down closes our server, which
// unlinks the path (libuv does, whatever it names now; #756), so the next
// round binds uncontested. What tells the two apart is whose socket the path
// names at the verdict: a window that confirmed on the competitor's
// socket is a broker nobody can reach.
expect(roles).toEqual([true]);
expect(sameSocketFile(await socketFileIdentity(path), theirs!)).toBe(false);
const peer = await openWindow(fakeWindow({ entries: [{ surfaceId: 'far-1' }] }));
await peer.ensurePeerNet(() => {});
expect(peer.isPeerBroker()).toBe(false);
} finally {
for (const socket of held) socket.destroy();
if (competitor.listening) await new Promise((resolve) => competitor.close(resolve));
}
}, 30_000);

it('collects directory entries from the other window', async () => {
const peerSide = fakeWindow({ entries: [{ surfaceId: 'far-1' }, { surfaceId: 'far-2' }] });
const { broker } = await linkedPair(fakeWindow(), peerSide);
Expand Down
Loading