Skip to content

Commit afba87a

Browse files
PR Batch Testerclaude
andcommitted
fix(desktop): skip guests destroyed while waiting to open a control session
A guest destroyed while another guest held the control-session lock threw from isDevToolsOpened once it got the lock. prepareWebview runs detached from the attach event, so that defect became an unhandled rejection. Check for a destroyed guest first, and keep any attach-time failure inside prepareWebview. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
1 parent 654649d commit afba87a

2 files changed

Lines changed: 110 additions & 46 deletions

File tree

‎apps/desktop/src/preview/Manager.test.ts‎

Lines changed: 89 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1691,53 +1691,69 @@ describe("PreviewManager", () => {
16911691
),
16921692
);
16931693

1694+
const makeAttachingGuest = (id: number) => {
1695+
const listeners = new Map<string, () => void>();
1696+
const attach = vi.fn();
1697+
const detach = vi.fn();
1698+
const sendCommand = vi.fn<(method: string, params?: unknown) => Promise<unknown>>(
1699+
async () => undefined,
1700+
);
1701+
let destroyed = false;
1702+
const wc = {
1703+
id,
1704+
isDestroyed: () => destroyed,
1705+
isDevToolsOpened: () => {
1706+
// Electron throws from native methods once a WebContents is destroyed.
1707+
if (destroyed) throw new TypeError("Object has been destroyed");
1708+
return false;
1709+
},
1710+
getType: () => "webview",
1711+
getURL: () => "http://localhost:5173/README.md",
1712+
getTitle: () => "README.md",
1713+
isLoading: () => true,
1714+
getZoomFactor: () => 1,
1715+
setZoomFactor: vi.fn(),
1716+
setAudioMuted: vi.fn(),
1717+
isCurrentlyAudible: () => false,
1718+
on: vi.fn(),
1719+
off: vi.fn(),
1720+
once: (event: string, listener: () => void) => {
1721+
listeners.set(event, listener);
1722+
},
1723+
ipc: { on: vi.fn(), off: vi.fn() },
1724+
send: webviewSend,
1725+
navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1726+
setIgnoreMenuShortcuts: vi.fn(),
1727+
setWindowOpenHandler: vi.fn(),
1728+
debugger: {
1729+
isAttached: () => attach.mock.calls.length > detach.mock.calls.length,
1730+
attach,
1731+
detach,
1732+
sendCommand,
1733+
on: vi.fn(),
1734+
off: vi.fn(),
1735+
},
1736+
};
1737+
return {
1738+
wc: wc as unknown as Electron.WebContents,
1739+
attach,
1740+
detach,
1741+
sendCommand,
1742+
destroy: () => {
1743+
destroyed = true;
1744+
listeners.get("destroyed")?.();
1745+
},
1746+
};
1747+
};
1748+
16941749
effectIt.effect("sets the opaque base as soon as a guest attaches", () =>
16951750
withManager((manager) =>
16961751
Effect.gen(function* () {
1697-
const makeGuest = (id: number) => {
1698-
const listeners = new Map<string, () => void>();
1699-
const attach = vi.fn();
1700-
const detach = vi.fn();
1701-
const sendCommand = vi.fn(async () => undefined);
1702-
const wc = {
1703-
id,
1704-
isDestroyed: () => false,
1705-
isDevToolsOpened: () => false,
1706-
getType: () => "webview",
1707-
getURL: () => "http://localhost:5173/README.md",
1708-
getTitle: () => "README.md",
1709-
isLoading: () => true,
1710-
getZoomFactor: () => 1,
1711-
setZoomFactor: vi.fn(),
1712-
setAudioMuted: vi.fn(),
1713-
isCurrentlyAudible: () => false,
1714-
on: vi.fn(),
1715-
off: vi.fn(),
1716-
once: (event: string, listener: () => void) => {
1717-
listeners.set(event, listener);
1718-
},
1719-
ipc: { on: vi.fn(), off: vi.fn() },
1720-
send: webviewSend,
1721-
navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1722-
setIgnoreMenuShortcuts: vi.fn(),
1723-
setWindowOpenHandler: vi.fn(),
1724-
debugger: {
1725-
isAttached: () => attach.mock.calls.length > detach.mock.calls.length,
1726-
attach,
1727-
detach,
1728-
sendCommand,
1729-
on: vi.fn(),
1730-
off: vi.fn(),
1731-
},
1732-
};
1733-
return { wc, attach, detach, sendCommand, listeners };
1734-
};
1735-
17361752
// The tab's first document can paint before the renderer registers the
17371753
// guest, so the opaque base has to be the first command on attach.
1738-
const claimed = makeGuest(44);
1739-
fromId.mockReturnValue(claimed.wc as never);
1740-
yield* manager.prepareWebview(claimed.wc as never);
1754+
const claimed = makeAttachingGuest(44);
1755+
fromId.mockReturnValue(claimed.wc);
1756+
yield* manager.prepareWebview(claimed.wc);
17411757
expect(claimed.attach).toHaveBeenCalledTimes(1);
17421758
expect(claimed.sendCommand.mock.calls[0]).toEqual([
17431759
"Emulation.setDefaultBackgroundColorOverride",
@@ -1753,17 +1769,45 @@ describe("PreviewManager", () => {
17531769
});
17541770

17551771
// A guest destroyed before any tab claims it releases its session.
1756-
const unclaimed = makeGuest(45);
1757-
yield* manager.prepareWebview(unclaimed.wc as never);
1772+
const unclaimed = makeAttachingGuest(45);
1773+
yield* manager.prepareWebview(unclaimed.wc);
17581774
expect(unclaimed.attach).toHaveBeenCalledTimes(1);
1759-
unclaimed.listeners.get("destroyed")?.();
1775+
unclaimed.destroy();
17601776
yield* Effect.yieldNow;
17611777
expect(unclaimed.detach).toHaveBeenCalledTimes(1);
17621778
expect(claimed.detach).not.toHaveBeenCalled();
17631779
}),
17641780
),
17651781
);
17661782

1783+
effectIt.effect("skips a guest destroyed while another guest's session opens", () =>
1784+
withManager((manager) =>
1785+
Effect.gen(function* () {
1786+
const slow = makeAttachingGuest(46);
1787+
let releaseSlow = () => {};
1788+
const slowCommand = new Promise<void>((resolve) => {
1789+
releaseSlow = resolve;
1790+
});
1791+
slow.sendCommand.mockImplementation(() => slowCommand);
1792+
const queued = makeAttachingGuest(47);
1793+
1794+
const slowFiber = yield* manager
1795+
.prepareWebview(slow.wc)
1796+
.pipe(Effect.forkChild({ startImmediately: true }));
1797+
const queuedFiber = yield* manager
1798+
.prepareWebview(queued.wc)
1799+
.pipe(Effect.forkChild({ startImmediately: true }));
1800+
queued.destroy();
1801+
releaseSlow();
1802+
1803+
expect(Exit.isSuccess(yield* Fiber.await(slowFiber))).toBe(true);
1804+
expect(Exit.isSuccess(yield* Fiber.await(queuedFiber))).toBe(true);
1805+
expect(slow.attach).toHaveBeenCalledTimes(1);
1806+
expect(queued.attach).not.toHaveBeenCalled();
1807+
}),
1808+
),
1809+
);
1810+
17671811
const makeAudioWebContents = (id: number) => {
17681812
const listeners = new Map<string, (...args: never[]) => void>();
17691813
const setAudioMuted = vi.fn();

‎apps/desktop/src/preview/Manager.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1303,6 +1303,17 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
13031303
> => {
13041304
const existing = sessions.get(wc.id);
13051305
if (existing) return Effect.succeed([existing, sessions] as const);
1306+
// A guest can be destroyed while it waits for this lock, and its native
1307+
// methods throw once it is.
1308+
if (wc.isDestroyed()) {
1309+
return Effect.fail(
1310+
new PreviewOperationError({
1311+
operation: "ensureControlSession",
1312+
webContentsId: wc.id,
1313+
cause: new Error("WebContents was destroyed"),
1314+
}),
1315+
);
1316+
}
13061317
if (wc.isDevToolsOpened()) {
13071318
return Effect.fail(
13081319
new PreviewAutomationDevToolsOpenError({
@@ -2429,7 +2440,16 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
24292440
wc.once("destroyed", () => {
24302441
runFork(detachControlSession(webContentsId));
24312442
});
2432-
yield* ensureControlSession(wc).pipe(Effect.ignore);
2443+
// Runs detached from the attach event, so nothing may escape. registerWebview
2444+
// opens the session again if this one did not.
2445+
yield* ensureControlSession(wc).pipe(
2446+
Effect.catchCause((cause) =>
2447+
Effect.logDebug("Preview webview control session was not opened on attach.", {
2448+
webContentsId,
2449+
cause,
2450+
}),
2451+
),
2452+
);
24332453
});
24342454

24352455
const navigate = Effect.fn("PreviewManager.navigate")(function* (tabId: string, rawUrl: string) {

0 commit comments

Comments
 (0)