net: throw on multiple listen calls

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #64871
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>

Author: mcollina

lib/net.js

@@ -2256,6 +2256,7 @@ function Server(options, connectionListener) {
 
   this[async_id_symbol] = -1;
   this._handle = null;
+  this._listening = false;
   this._usingWorkers = false;
   this._workers = [];
   this._unref = false;
@@ -2476,6 +2477,7 @@ Server.prototype[kTransfer] = function() {
   };
   // Detach so the source server no longer references the handle being moved.
   this._handle = null;
+  this._listening = false;
   return {
     data,
     deserializeInfo: 'net:Server',
@@ -2499,6 +2501,7 @@ Server.prototype[kDeserialize] = function(data) {
 };
 
 function emitErrorNT(self, err) {
+  self._listening = false;
   self.emit('error', err);
 }
 
@@ -2544,6 +2547,7 @@ function listenInCluster(server, address, port, addressType,
 
     if (err) {
       const ex = new ExceptionWithHostPort(err, 'bind', address, port);
+      server._listening = false;
       return server.emit('error', ex);
     }
     // If there was a handle, just close it to avoid fd leak
@@ -2564,7 +2568,7 @@ Server.prototype.listen = function(...args) {
   let options = normalized[0];
   const cb = normalized[1];
 
-  if (this._handle) {
+  if (this._handle || this._listening) {
     throw new ERR_SERVER_ALREADY_LISTEN();
   }
 
@@ -2596,6 +2600,7 @@ Server.prototype.listen = function(...args) {
       this._pipeName = boundPath;
     }
     this[async_id_symbol] = this._handle.getAsyncId();
+    this._listening = true;
     this._listeningId++;
     listenInCluster(this, null, -1, -1, backlogFromArgs, undefined, true);
     return this;
@@ -2608,12 +2613,14 @@ Server.prototype.listen = function(...args) {
   if (options instanceof TCP) {
     this._handle = options;
     this[async_id_symbol] = this._handle.getAsyncId();
+    this._listening = true;
     listenInCluster(this, null, -1, -1, backlogFromArgs, undefined, true);
     return this;
   }
   addServerAbortSignalOption(this, options);
   // (handle[, backlog][, cb]) where handle is an object with a fd
   if (typeof options.fd === 'number' && options.fd >= 0) {
+    this._listening = true;
     listenInCluster(this, null, null, null, backlogFromArgs, options.fd);
     return this;
   }
@@ -2638,6 +2645,7 @@ Server.prototype.listen = function(...args) {
       options.exclusive = true;
     }
     // start TCP server listening on host:port
+    this._listening = true;
     if (options.host) {
       lookupAndListen(this, options.port | 0, options.host, backlog,
                       options.exclusive, flags);
@@ -2660,6 +2668,7 @@ Server.prototype.listen = function(...args) {
     }
     const pipeName = this._pipeName = options.path;
     backlog = options.backlog || backlogFromArgs;
+    this._listening = true;
     listenInCluster(this,
                     pipeName,
                     -1,
@@ -2689,6 +2698,7 @@ Server.prototype.listen = function(...args) {
       if (err) {
         this._handle.close();
         this._handle = null;
+        this._listening = false;
         throw new ErrnoException(err, 'uv_pipe_chmod');
       }
     }
@@ -2742,6 +2752,7 @@ function lookupAndListen(self, port, address, backlog,
       return;
     }
     if (err) {
+      self._listening = false;
       self.emit('error', err);
     } else {
       const validAddress = filterOnlyValidAddress(addresses);
@@ -2896,6 +2907,7 @@ Server.prototype.getConnections = function(cb) {
 
 
 Server.prototype.close = function(cb) {
+  this._listening = false;
   this._listeningId++;
   if (typeof cb === 'function') {
     if (!this._handle) {

test/parallel/test-net-listen-twice.js

@@ -11,28 +11,12 @@ if (cluster.isPrimary) {
   }));
 } else {
   const server = net.createServer();
-  server.listen();
-  try {
-    // Currently, we can call `listen` twice in cluster worker,
-    // if we can not call `listen` twice in the future,
-    // just skip this test.
-    server.listen();
-  } catch (e) {
-    console.error(e);
-    return;
-  }
-  let i = 0;
-  process.on('internalMessage', (msg) => {
-    if (msg.cmd === 'NODE_CLUSTER') {
-      if (++i === 2) {
-        setImmediate(() => {
-          server.close(() => {
-            process.disconnect();
-          });
-        });
-      }
-    }
+  server.listen(common.mustCall(() => {
+    server.close(() => process.disconnect());
+  }));
+
+  assert.throws(() => server.listen(), {
+    code: 'ERR_SERVER_ALREADY_LISTEN',
+    name: 'Error'
   });
-  // Must only call once
-  server.on('listening', common.mustCall());
 }

test/parallel/test-net-server-call-listen-multiple-times.js

@@ -35,7 +35,21 @@ const net = require('net');
   });
 }
 
-// Third test.
+// Third test. Check that a second listen call throws while the first is pending.
+{
+  const server = net.Server();
+
+  server.listen(0, '127.0.0.1');
+
+  assert.throws(() => server.listen(), {
+    code: 'ERR_SERVER_ALREADY_LISTEN',
+    name: 'Error'
+  });
+
+  server.close();
+}
+
+// Fourth test.
 // Check that after the close call you can run listen method just fine.
 {
   const server = net.Server();