Skip to content

dgram: remove listeners on bind error - #30210

Closed
addaleax wants to merge 3 commits into
nodejs:masterfrom
addaleax:dgram-bind-error
Closed

dgram: remove listeners on bind error#30210
addaleax wants to merge 3 commits into
nodejs:masterfrom
addaleax:dgram-bind-error

Conversation

@addaleax

@addaleaxaddaleax commented Nov 1, 2019

Copy link
Copy Markdown
Member
dgram: reset bind state before emitting error

This was previously done inconsistently, sometimes before, sometimes
after emitting the event.

dgram: remove listeners on bind error

This avoids piling up 'listening' event listeners if
.bind() fails repeatedly.

Fixes: #30209

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

@addaleax
dgram: reset bind state before emitting error
e17fcc8
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.
@addaleax
dgram: remove listeners on bind error
2d8aa64
This avoids piling up `'listening'` event listeners if
`.bind()` fails repeatedly.
Fixes: nodejs#30209
@nodejs-github-botnodejs-github-bot added the dgram Issues and PRs related to the dgram subsystem / UDP. label Nov 1, 2019
@nodejs-github-bot

nodejs-github-bot commented Nov 1, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26285/

lpinca
lpinca approved these changes Nov 2, 2019
View reviewed changes
Comment threadtest/parallel/test-dgram-bind-error-repeat.js Outdated
@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 5, 2019
@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26353/

@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26360/

@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26373/

@nodejs-github-bot

nodejs-github-bot commented Nov 6, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26404/

@nodejs-github-bot

nodejs-github-bot commented Nov 6, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26415/

@nodejs-github-bot

nodejs-github-bot commented Nov 6, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26421/

@addaleax

addaleax commented Nov 6, 2019

Copy link
Copy Markdown
MemberAuthor

Landed in 99e874e115792d

@addaleaxaddaleax closed this Nov 6, 2019
addaleax added a commit that referenced this pull request Nov 6, 2019
@addaleax
dgram: reset bind state before emitting error
99e874e
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.
PR-URL: #30210Fixes: #30209
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this pull request Nov 6, 2019
@addaleax
dgram: remove listeners on bind error
115792d
This avoids piling up `'listening'` event listeners if
`.bind()` fails repeatedly.
Fixes: #30209
PR-URL: #30210
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax
addaleax deleted the dgram-bind-error branch November 6, 2019 23:55
@richardlaurichardlau mentioned this pull request Nov 7, 2019
Closed
4 tasks
MylesBorins pushed a commit that referenced this pull request Nov 17, 2019
@addaleax@MylesBorins
dgram: reset bind state before emitting error
0433d79
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.
PR-URL: #30210Fixes: #30209
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 17, 2019
@addaleax@MylesBorins
dgram: remove listeners on bind error
f8ee70c
This avoids piling up `'listening'` event listeners if
`.bind()` fails repeatedly.
Fixes: #30209
PR-URL: #30210
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeARBridgeAR mentioned this pull request Nov 19, 2019
Merged
targos pushed a commit that referenced this pull request Dec 1, 2019
@addaleax@targos
dgram: reset bind state before emitting error
aa30eb5
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.
PR-URL: #30210Fixes: #30209
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Dec 1, 2019
@addaleax@targos
dgram: remove listeners on bind error
29c8199
This avoids piling up `'listening'` event listeners if
`.bind()` fails repeatedly.
Fixes: #30209
PR-URL: #30210
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Dec 9, 2019
Closed
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
@addaleax@MylesBorins
dgram: reset bind state before emitting error
fd5fed1
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.
PR-URL: #30210Fixes: #30209
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
@addaleax@MylesBorins
dgram: remove listeners on bind error
3d39be7
This avoids piling up `'listening'` event listeners if
`.bind()` fails repeatedly.
Fixes: #30209
PR-URL: #30210
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Dec 23, 2019
Merged
@coderabbitaicoderabbitaiBot mentioned this pull request Jul 25, 2026
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@lpincalpincalpinca approved these changes
@cjihrigcjihrigcjihrig approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.dgramIssues and PRs related to the dgram subsystem / UDP.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

EventEmitter memory leak with socket bind

7 participants

@addaleax@nodejs-github-bot@jasnell@lpinca@cjihrig@MylesBorins@BethGriggs