From 97aaa12be46f211d94559c8dcadc33052c45bac2 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 7 Oct 2024 18:43:25 +0200 Subject: [PATCH] cherry-pick(#32956): fix(fetch): listener leaks on Socket Closes https://github.com/microsoft/playwright/issues/32951 `node:http` reuses TCP Sockets under the hood. We weren't cleaning up our listeners, leading to the `MaxListenersExceededWarning`. This PR adds cleanup logic. It also raises the warning threshhold, so that it doesn't trigger until there's 100 concurrent requests over the same socket. --- packages/playwright-core/src/server/fetch.ts | 52 +++++++++++--------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index f07f8c63f343c..9505dab28fded 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -25,7 +25,7 @@ import zlib from 'zlib'; import type { HTTPCredentials } from '../../types/types'; import { TimeoutSettings } from '../common/timeoutSettings'; import { getUserAgent } from '../utils/userAgent'; -import { assert, createGuid, monotonicTime } from '../utils'; +import { assert, createGuid, eventsHelper, monotonicTime, type RegisteredListener } from '../utils'; import { HttpsProxyAgent, SocksProxyAgent } from '../utilsBundle'; import { BrowserContext, verifyClientCertificates } from './browserContext'; import { CookieStore, domainMatches, parseRawCookie } from './cookieStore'; @@ -312,8 +312,11 @@ export abstract class APIRequestContext extends SdkObject { let securityDetails: har.SecurityDetails | undefined; + const listeners: RegisteredListener[] = []; + const request = requestConstructor(url, requestOptions as any, async response => { const responseAt = monotonicTime(); + const notifyRequestFinished = (body?: Buffer) => { const endAt = monotonicTime(); // spec: http://www.softwareishard.com/blog/har-12-spec/#timings @@ -478,12 +481,13 @@ export abstract class APIRequestContext extends SdkObject { }); request.on('error', reject); - const disposeListener = () => { - reject(new Error('Request context disposed.')); - request.destroy(); - }; - this.on(APIRequestContext.Events.Dispose, disposeListener); - request.on('close', () => this.off(APIRequestContext.Events.Dispose, disposeListener)); + listeners.push( + eventsHelper.addEventListener(this, APIRequestContext.Events.Dispose, () => { + reject(new Error('Request context disposed.')); + request.destroy(); + }) + ); + request.on('close', () => eventsHelper.removeEventListeners(listeners)); request.on('socket', socket => { // happy eyeballs don't emit lookup and connect events, so we use our custom ones @@ -492,22 +496,24 @@ export abstract class APIRequestContext extends SdkObject { tcpConnectionAt = happyEyeBallsTimings.tcpConnectionAt; // non-happy-eyeballs sockets - socket.on('lookup', () => { dnsLookupAt = monotonicTime(); }); - socket.on('connect', () => { tcpConnectionAt = monotonicTime(); }); - socket.on('secureConnect', () => { - tlsHandshakeAt = monotonicTime(); - - if (socket instanceof TLSSocket) { - const peerCertificate = socket.getPeerCertificate(); - securityDetails = { - protocol: socket.getProtocol() ?? undefined, - subjectName: peerCertificate.subject.CN, - validFrom: new Date(peerCertificate.valid_from).getTime() / 1000, - validTo: new Date(peerCertificate.valid_to).getTime() / 1000, - issuer: peerCertificate.issuer.CN - }; - } - }); + listeners.push( + eventsHelper.addEventListener(socket, 'lookup', () => { dnsLookupAt = monotonicTime(); }), + eventsHelper.addEventListener(socket, 'connect', () => { tcpConnectionAt = monotonicTime(); }), + eventsHelper.addEventListener(socket, 'secureConnect', () => { + tlsHandshakeAt = monotonicTime(); + + if (socket instanceof TLSSocket) { + const peerCertificate = socket.getPeerCertificate(); + securityDetails = { + protocol: socket.getProtocol() ?? undefined, + subjectName: peerCertificate.subject.CN, + validFrom: new Date(peerCertificate.valid_from).getTime() / 1000, + validTo: new Date(peerCertificate.valid_to).getTime() / 1000, + issuer: peerCertificate.issuer.CN + }; + } + }), + ); serverIPAddress = socket.remoteAddress; serverPort = socket.remotePort;