Skip to content

http2: Prevent unnecessary listeners from being registered - #27987

Closed
akukas wants to merge 6 commits into
nodejs:masterfrom
akukas:patch-1
Closed

http2: Prevent unnecessary listeners from being registered#27987
akukas wants to merge 6 commits into
nodejs:masterfrom
akukas:patch-1

Conversation

@akukas

Copy link
Copy Markdown
Contributor

When multiple requests are started on a client HTTP/2 session while it it still connecting, a warning might get emitted: MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 connect listeners added to Http2Session

This PR modifies the offending code so that only one event listener is registered, no matter how many requests are started. A test for the fix is included.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the http2 Issues or PRs related to the http2 subsystem. label May 30, 2019
Comment threadlib/internal/http2/core.js Outdated
Comment threadlib/internal/http2/core.js Outdated
Comment threadlib/internal/http2/core.js Outdated
Comment threadlib/internal/http2/core.js Outdated
Update symbol description to match the name.
Initialize field in the constructor.
Reset field instead of deleting it.
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 4, 2019
@addaleax

Copy link
Copy Markdown
Member

Landed in bcf1135

addaleax pushed a commit that referenced this pull request Jun 10, 2019
PR-URL: #27987
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
PR-URL: #27987
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeARBridgeAR mentioned this pull request Jun 17, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.http2Issues or PRs related to the http2 subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@akukas@nodejs-github-bot@addaleax@mscdex@jasnell@BridgeAR