Skip to content

Commit 7efc22c

Browse files
authored
fix(chromium): websocket wrapper leaks child sessions (#2291)
When a parent session is detached, we do not always get Target.detachedFromTarget for child sessions. This is especially true when the socket disconnects, leaving all child sessions in the maps. Flakily reproducible by browserType.connect multiclient tests.
1 parent 4816434 commit 7efc22c

File tree

1 file changed

+37
-13
lines changed

1 file changed

+37
-13
lines changed

src/server/chromium.ts

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -208,31 +208,56 @@ export class Chromium extends AbstractBrowserType<CRBrowser> {
208208
}
209209
}
210210

211+
type SessionData = {
212+
socket: ws,
213+
children: Set<string>,
214+
isBrowserSession: boolean,
215+
parent?: string,
216+
};
217+
211218
function wrapTransportWithWebSocket(transport: ConnectionTransport, logger: InnerLogger, port: number): WebSocketWrapper {
212219
const server = new ws.Server({ port });
213220
const guid = helper.guid();
214221

215222
const awaitingBrowserTarget = new Map<number, ws>();
216-
const sessionToSocket = new Map<string, ws>();
223+
const sessionToData = new Map<string, SessionData>();
217224
const socketToBrowserSession = new Map<ws, { sessionId?: string, queue?: ProtocolRequest[] }>();
218-
const browserSessions = new Set<string>();
219225
let lastSequenceNumber = 1;
220226

227+
function addSession(sessionId: string, socket: ws, parentSessionId?: string) {
228+
sessionToData.set(sessionId, {
229+
socket,
230+
children: new Set(),
231+
isBrowserSession: !parentSessionId,
232+
parent: parentSessionId
233+
});
234+
if (parentSessionId)
235+
sessionToData.get(parentSessionId)!.children.add(sessionId);
236+
}
237+
238+
function removeSession(sessionId: string) {
239+
const data = sessionToData.get(sessionId)!;
240+
for (const child of data.children)
241+
removeSession(child);
242+
if (data.parent)
243+
sessionToData.get(data.parent)!.children.delete(sessionId);
244+
sessionToData.delete(sessionId);
245+
}
246+
221247
transport.onmessage = message => {
222248
if (typeof message.id === 'number' && awaitingBrowserTarget.has(message.id)) {
223249
const freshSocket = awaitingBrowserTarget.get(message.id)!;
224250
awaitingBrowserTarget.delete(message.id);
225251

226252
const sessionId = message.result.sessionId;
227253
if (freshSocket.readyState !== ws.CLOSED && freshSocket.readyState !== ws.CLOSING) {
228-
sessionToSocket.set(sessionId, freshSocket);
229254
const { queue } = socketToBrowserSession.get(freshSocket)!;
230255
for (const item of queue!) {
231256
item.sessionId = sessionId;
232257
transport.send(item);
233258
}
234259
socketToBrowserSession.set(freshSocket, { sessionId });
235-
browserSessions.add(sessionId);
260+
addSession(sessionId, freshSocket);
236261
} else {
237262
transport.send({
238263
id: ++lastSequenceNumber,
@@ -248,16 +273,16 @@ function wrapTransportWithWebSocket(transport: ConnectionTransport, logger: Inne
248273
if (!message.sessionId)
249274
return;
250275

251-
const socket = sessionToSocket.get(message.sessionId);
252-
if (socket && socket.readyState !== ws.CLOSING) {
276+
const data = sessionToData.get(message.sessionId);
277+
if (data && data.socket.readyState !== ws.CLOSING) {
253278
if (message.method === 'Target.attachedToTarget')
254-
sessionToSocket.set(message.params.sessionId, socket);
279+
addSession(message.params.sessionId, data.socket, message.sessionId);
255280
if (message.method === 'Target.detachedFromTarget')
256-
sessionToSocket.delete(message.params.sessionId);
281+
removeSession(message.params.sessionId);
257282
// Strip session ids from the browser sessions.
258-
if (browserSessions.has(message.sessionId))
283+
if (data.isBrowserSession)
259284
delete message.sessionId;
260-
socket.send(JSON.stringify(message));
285+
data.socket.send(JSON.stringify(message));
261286
}
262287
};
263288

@@ -311,8 +336,7 @@ function wrapTransportWithWebSocket(transport: ConnectionTransport, logger: Inne
311336
const session = socketToBrowserSession.get(socket);
312337
if (!session || !session.sessionId)
313338
return;
314-
sessionToSocket.delete(session.sessionId);
315-
browserSessions.delete(session.sessionId);
339+
removeSession(session.sessionId);
316340
socketToBrowserSession.delete(socket);
317341
transport.send({
318342
id: ++lastSequenceNumber,
@@ -324,7 +348,7 @@ function wrapTransportWithWebSocket(transport: ConnectionTransport, logger: Inne
324348

325349
const address = server.address();
326350
const wsEndpoint = typeof address === 'string' ? `${address}/${guid}` : `ws://127.0.0.1:${address.port}/${guid}`;
327-
return new WebSocketWrapper(wsEndpoint, [awaitingBrowserTarget, sessionToSocket, socketToBrowserSession, browserSessions]);
351+
return new WebSocketWrapper(wsEndpoint, [awaitingBrowserTarget, sessionToData, socketToBrowserSession]);
328352
}
329353

330354

0 commit comments

Comments
 (0)