From 15206386f1780ed6813156b3d77be9aa2b626223 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Mon, 29 Jul 2024 12:42:12 -0400 Subject: [PATCH] tools: Fix EventEmitter leak in await-packagist-updates.mjs (#38505) Each time through the loop we're adding a listener on the http2Client and never removing it. Fix that. --- tools/js-tools/await-packagist-updates.mjs | 23 ++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tools/js-tools/await-packagist-updates.mjs b/tools/js-tools/await-packagist-updates.mjs index 47678a7bf877a..4f38aaafc4c3c 100755 --- a/tools/js-tools/await-packagist-updates.mjs +++ b/tools/js-tools/await-packagist-updates.mjs @@ -80,7 +80,7 @@ function pollPackagist( name, versionRange ) { task: async ( ctx, task ) => { while ( ! aborter.aborted ) { try { - let req; + const cleanup = []; // http2 isn't a promise-based API, so wrap in a promise ourselves and await it. const done = await new Promise( ( resolve, reject ) => { // Reject the promise with a fatal-flagged error. @@ -112,9 +112,22 @@ function pollPackagist( name, versionRange ) { // So listen for that just in case, because hung promises are bad. // @see https://nodejs.org/api/http2.html#error-handling http2Client.on( 'error', reject ); + cleanup.push( () => { + if ( http2Client ) { + http2Client.off( 'error', reject ); + } + } ); // Make the actual request. - req = http2Client.request( reqHeaders, { signal: aborter.signal } ); + const req = http2Client.request( reqHeaders, { signal: aborter.signal } ); + cleanup.push( () => { + // Make sure the request actually gets closed whenever the promise settles. + // We don't want hung connections waiting on us to read more data or something. + if ( ! req.closed && ! req.destroyed ) { + req.close( http2.constants.NGHTTP2_CANCEL ); + } + } ); + req.on( 'error', reject ); req.on( 'response', resHeaders => { const status = resHeaders[ http2.constants.HTTP2_HEADER_STATUS ]; @@ -178,10 +191,8 @@ function pollPackagist( name, versionRange ) { } ); req.end(); } ).finally( () => { - // Make sure the request actually gets closed whenever the promise settles. - // We don't want hung connections waiting on us to read more data or something. - if ( req && ! req.closed && ! req.destroyed ) { - req.close( http2.constants.NGHTTP2_CANCEL ); + for ( const cb of cleanup ) { + cb(); } } );