Crashes when UDP/dgram socket is closed during listening event handler

[Matthias247]

• Version: 4.3, master
• Platform: Linux 4.4
• Subsystem: dgram

Node.js crashes when a UDP/dgram socket is closed in the listening event handler.

Output / Stack trace using node.js 4.3:

dgram.js:460
    throw new Error('Not running'); // error message from dgram_legacy.js
    ^

Error: Not running
at Socket._healthCheck (dgram.js:460:11)
at Socket.send (dgram.js:284:8)
at Socket.<anonymous> (dgram.js:299:21)
at Socket.g (events.js:260:16)
at emitNone (events.js:72:20)
at Socket.emit (events.js:166:7)
at startListening (dgram.js:121:10)
at dgram.js:220:7
at nextTickCallbackWith3Args (node.js:452:9)
at process._tickCallback (node.js:358:17)

This seems to happen because node.js attaches an own eventhandler to the listening event which will try to send messages that were enqueued before, but have not been sent (https://github.com/nodejs/node/blob/master/lib/dgram.js#L288-L293). This closure might run after a user event handler which might close the socket (like mine does if it fails to join a multicast group). When it runs and tries to send messages on the already closed socket an exception is thrown which can not be captured by the user, because it's not on his callstack.

What should be done to avoid that:

• The listening handler should check if the socket is still open before starting to send enqueued messages and should simply return if it's no longer open.
• The close() function should flush the sendQueue, which would also avoid that the handler does attempt to send anything.

Actually the second part is more part, because it seems that currently the callbacks for those queued sends are also never called if the socket is closed before connecting and because it would already solve the issue. However another sanity check in the listening handler would still be ok.

Workarounds:

• Don't send anything before listening is emitted
• Don't close the socket or do any complex logic in the listening handler, but defer the custom logic in a new eventloop tick.

My code for reproduction:

class MulticastSocket {
// ...

open() {
  this._socket = dgram.createSocket(<any>{
    type: 'udp4',
    reuseAddr: true,
  });

  this._socket.once('listening', () => {
    console.log('Multicast socket is listening')     

    try {
      // Try to join a multicast group
      // This might fail depending on the network interfaces of the device
      this._socket.addMembership(this._multicastAddress);
      this._state = 'CONNECTED';
    } catch (e) {
      console.error('Adding multicast group membership failed')        
      this._closeSocket(); // The socket is closed here inside the event listener
    }
  });
}

_closeSocket() {
  if (this._state === 'CLOSED') return false;
  this._state = 'CLOSED';

  if (this._socket) {
  this._socket.removeAllListeners();
  this._socket.close();
  this._socket = null;
}

[Fishrock123]

cc @mcollina

[mcollina]

Thanks for reporting.

It is a bug, as the sendQueue is not flushed out before calling close if this happens before a 'listening' event handler is specified.
I will try to propose a fix during this week.