Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EventEmitter memory leak with socket bind #30209

Closed
karlsnyder0 opened this issue Nov 1, 2019 · 0 comments
Closed

EventEmitter memory leak with socket bind #30209

karlsnyder0 opened this issue Nov 1, 2019 · 0 comments
Labels
confirmed-bug Issues with confirmed bugs. dgram Issues and PRs related to the dgram subsystem / UDP.

Comments

@karlsnyder0
Copy link

  • Version: v8.16.0 and greater
  • Platform: MacOS
  • Subsystem: dgram socket

When binding to a broadcast socket using-

const doSocketBind = () => {
    this.socket.bind(broadcastPort, undefined, () => {
        console.log('Success');
    });
};

And retrying the bind after a failure-

this.socket.on('error', (err: Error) => {
    if (err['code'] === 'EADDRINUSE') {
        setTimeout(() => {
            doSocketBind();
        }, 5000);
    }
});

After a few failures I receive this message-

(node:68039) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 listening listeners added. Use emitter.setMaxListeners() to increase limit

For example, if something else has bound to the same broadcast port and has locked the port.

The issue is caused by-

this.once('listening', arguments[arguments.length - 1]);

arguments[arguments.length - 1] is intended to be the callback. In the case of a failure, the callback doesn't execute and the callback remains in the event queue waiting to be executed. If there are 10 bind failures the callback will execute 10 times after being able to successfully bind in the 11th retry.

A workaround-

Define the callback as a named function and remove the callback if an error is encountered.

this.socket.on('error', (err: Error) => {
    this.socket.removeListener('listening', onSocketBind);

    if (err['code'] === 'EADDRINUSE') {
        setTimeout(() => {
            doSocketBind();
        }, 5000);
    }
});
@addaleax addaleax added dgram Issues and PRs related to the dgram subsystem / UDP. confirmed-bug Issues with confirmed bugs. labels Nov 1, 2019
addaleax added a commit to addaleax/node that referenced this issue Nov 1, 2019
This avoids piling up `'listening'` event listeners if
`.bind()` fails repeatedly.

Fixes: nodejs#30209
addaleax added a commit that referenced this issue Nov 6, 2019
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>
MylesBorins pushed a commit that referenced this issue Nov 17, 2019
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.

PR-URL: #30210
Fixes: #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 issue Nov 17, 2019
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>
targos pushed a commit that referenced this issue Dec 1, 2019
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.

PR-URL: #30210
Fixes: #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 issue Dec 1, 2019
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>
MylesBorins pushed a commit that referenced this issue Dec 17, 2019
This was previously done inconsistently, sometimes before, sometimes
after emitting the event.

PR-URL: #30210
Fixes: #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 issue Dec 17, 2019
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. dgram Issues and PRs related to the dgram subsystem / UDP.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants