http2: do not register unnecessary listeners

PR-URL: #27987 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jun 17, 2019 · e1fc9b9 · e1fc9b9
1 parent 79a8cd0
commit e1fc9b9
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 1 deletion.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -161,6 +161,7 @@ const kLocalSettings = Symbol('local-settings');
 const kOptions = Symbol('options');
 const kOwner = owner_symbol;
 const kOrigin = Symbol('origin');
+const kPendingRequestCalls = Symbol('kPendingRequestCalls');
 const kProceed = Symbol('proceed');
 const kProtocol = Symbol('protocol');
 const kRemoteSettings = Symbol('remote-settings');
@@ -1417,6 +1418,7 @@ class ServerHttp2Session extends Http2Session {
 class ClientHttp2Session extends Http2Session {
   constructor(options, socket) {
     super(NGHTTP2_SESSION_CLIENT, options, socket);
+    this[kPendingRequestCalls] = null;
   }
 
   // Submits a new HTTP2 request to the connected peer. Returns the
@@ -1486,7 +1488,15 @@ class ClientHttp2Session extends Http2Session {
 
     const onConnect = requestOnConnect.bind(stream, headersList, options);
     if (this.connecting) {
-      this.once('connect', onConnect);
+      if (this[kPendingRequestCalls] !== null) {
+        this[kPendingRequestCalls].push(onConnect);
+      } else {
+        this[kPendingRequestCalls] = [onConnect];
+        this.once('connect', () => {
+          this[kPendingRequestCalls].forEach((f) => f());
+          this[kPendingRequestCalls] = null;
+        });
+      }
     } else {
       onConnect();
     }

diff --git a/test/parallel/test-http2-client-request-listeners-warning.js b/test/parallel/test-http2-client-request-listeners-warning.js
@@ -0,0 +1,41 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const http2 = require('http2');
+const EventEmitter = require('events');
+
+// This test ensures that a MaxListenersExceededWarning isn't emitted if
+// more than EventEmitter.defaultMaxListeners requests are started on a
+// ClientHttp2Session before it has finished connecting.
+
+process.on('warning', common.mustNotCall('A warning was emitted'));
+
+const server = http2.createServer();
+server.on('stream', (stream) => {
+  stream.respond();
+  stream.end();
+});
+
+server.listen(common.mustCall(() => {
+  const client = http2.connect(`http://localhost:${server.address().port}`);
+
+  function request() {
+    return new Promise((resolve, reject) => {
+      const stream = client.request();
+      stream.on('error', reject);
+      stream.on('response', resolve);
+      stream.end();
+    });
+  }
+
+  const requests = [];
+  for (let i = 0; i < EventEmitter.defaultMaxListeners + 1; i++) {
+    requests.push(request());
+  }
+
+  Promise.all(requests).then(common.mustCall()).finally(common.mustCall(() => {
+    server.close();
+    client.close();
+  }));
+}));