From e17fcc85410a5aa54fb3b227096b4999feaf28fe Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 1 Nov 2019 19:50:16 +0100 Subject: [PATCH 1/3] dgram: reset bind state before emitting error This was previously done inconsistently, sometimes before, sometimes after emitting the event. --- lib/dgram.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index d29d1e7b190f3a..35d414320a7e43 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -240,8 +240,8 @@ Socket.prototype.bind = function(port_, address_ /* , callback */) { }, (err) => { // Callback to handle error. const ex = errnoException(err, 'open'); - this.emit('error', ex); state.bindState = BIND_STATE_UNBOUND; + this.emit('error', ex); }); return this; } @@ -309,8 +309,8 @@ Socket.prototype.bind = function(port_, address_ /* , callback */) { }, (err) => { // Callback to handle error. const ex = exceptionWithHostPort(err, 'bind', ip, port); - this.emit('error', ex); state.bindState = BIND_STATE_UNBOUND; + this.emit('error', ex); }); } else { if (!state.handle) @@ -319,8 +319,8 @@ Socket.prototype.bind = function(port_, address_ /* , callback */) { const err = state.handle.bind(ip, port || 0, flags); if (err) { const ex = exceptionWithHostPort(err, 'bind', ip, port); - this.emit('error', ex); state.bindState = BIND_STATE_UNBOUND; + this.emit('error', ex); // Todo: close? return; } From 2d8aa6407fa8f098bb57f91dff4803031a426e43 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 1 Nov 2019 19:50:40 +0100 Subject: [PATCH 2/3] dgram: remove listeners on bind error This avoids piling up `'listening'` event listeners if `.bind()` fails repeatedly. Fixes: https://github.com/nodejs/node/issues/30209 --- lib/dgram.js | 17 +++++++++-- test/parallel/test-dgram-bind-error-repeat.js | 28 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-dgram-bind-error-repeat.js diff --git a/lib/dgram.js b/lib/dgram.js index 35d414320a7e43..94f44d520220ff 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -211,8 +211,21 @@ Socket.prototype.bind = function(port_, address_ /* , callback */) { state.bindState = BIND_STATE_BINDING; - if (arguments.length && typeof arguments[arguments.length - 1] === 'function') - this.once('listening', arguments[arguments.length - 1]); + const cb = arguments.length && arguments[arguments.length - 1]; + if (typeof cb === 'function') { + function removeListeners() { + this.removeListener('error', removeListeners); + this.removeListener('listening', onListening); + } + + function onListening() { + removeListeners.call(this); + cb.call(this); + } + + this.on('error', removeListeners); + this.on('listening', onListening); + } if (port instanceof UDP) { replaceHandle(this, port); diff --git a/test/parallel/test-dgram-bind-error-repeat.js b/test/parallel/test-dgram-bind-error-repeat.js new file mode 100644 index 00000000000000..d0fdae5729b4f2 --- /dev/null +++ b/test/parallel/test-dgram-bind-error-repeat.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common'); +const dgram = require('dgram'); + +// Regression test for https://github.com/nodejs/node/issues/30209 +// No warning should be emitted when re-trying `.bind()` on UDP sockets +// repeatedly. + +process.on('warning', common.mustNotCall()); + +const reservePortSocket = dgram.createSocket('udp4'); +reservePortSocket.bind(() => { + const { port } = reservePortSocket.address(); + + const newSocket = dgram.createSocket('udp4'); + + let errors = 0; + newSocket.on('error', common.mustCall(() => { + if (++errors < 20) { + const cb = errors === 20 ? common.mustCall() : common.mustNotCall(); + newSocket.bind(port, cb); + } else { + newSocket.close(); + reservePortSocket.close(); + } + }, 20)); + newSocket.bind(port, common.mustNotCall()); +}); From f603ff405d7253580436b64b4c0ad2bb2a787e12 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Nov 2019 22:22:33 +0100 Subject: [PATCH 3/3] fixup! dgram: remove listeners on bind error --- test/parallel/test-dgram-bind-error-repeat.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/parallel/test-dgram-bind-error-repeat.js b/test/parallel/test-dgram-bind-error-repeat.js index d0fdae5729b4f2..a520d30a519cd2 100644 --- a/test/parallel/test-dgram-bind-error-repeat.js +++ b/test/parallel/test-dgram-bind-error-repeat.js @@ -17,8 +17,7 @@ reservePortSocket.bind(() => { let errors = 0; newSocket.on('error', common.mustCall(() => { if (++errors < 20) { - const cb = errors === 20 ? common.mustCall() : common.mustNotCall(); - newSocket.bind(port, cb); + newSocket.bind(port, common.mustNotCall()); } else { newSocket.close(); reservePortSocket.close();