[fix] Emit the 'close' event after the server is closed

Ensure that `WebSocketServer.prototype.close()` does not emit a
`'close'` event prematurely if called while the internal HTTP/S server
is closing.

Author: lpinca

lib/websocket-server.js

@@ -16,6 +16,10 @@ const { GUID, kWebSocket } = require('./constants');
 
 const keyRegex = /^[+/0-9A-Za-z]{22}==$/;
 
+const RUNNING = 0;
+const CLOSING = 1;
+const CLOSED = 2;
+
 /**
  * Class representing a WebSocket server.
  *
@@ -108,6 +112,7 @@ class WebSocketServer extends EventEmitter {
     if (options.perMessageDeflate === true) options.perMessageDeflate = {};
     if (options.clientTracking) this.clients = new Set();
     this.options = options;
+    this._state = RUNNING;
   }
 
   /**
@@ -137,6 +142,14 @@ class WebSocketServer extends EventEmitter {
   close(cb) {
     if (cb) this.once('close', cb);
 
+    if (this._state === CLOSED) {
+      process.nextTick(emitClose, this);
+      return;
+    }
+
+    if (this._state === CLOSING) return;
+    this._state = CLOSING;
+
     //
     // Terminate all associated clients.
     //
@@ -154,7 +167,7 @@ class WebSocketServer extends EventEmitter {
       // Close the http server if it was internally created.
       //
       if (this.options.port != null) {
-        server.close(() => this.emit('close'));
+        server.close(emitClose.bind(undefined, this));
         return;
       }
     }
@@ -373,6 +386,7 @@ function addListeners(server, map) {
  * @private
  */
 function emitClose(server) {
+  server._state = CLOSED;
   server.emit('close');
 }
 

test/websocket-server.test.js

@@ -278,11 +278,45 @@ describe('WebSocketServer', () => {
       });
     });
 
-    it("emits the 'close' event", (done) => {
-      const wss = new WebSocket.Server({ noServer: true });
+    it("emits the 'close' event after the server closes", (done) => {
+      let serverCloseEventEmitted = false;
+
+      const wss = new WebSocket.Server({ port: 0 }, () => {
+        net.createConnection({ port: wss.address().port });
+      });
+
+      wss._server.on('connection', (socket) => {
+        wss.close();
+
+        //
+        // The server is closing. Ensure this does not emit a `'close'`
+        // event before the server is actually closed.
+        //
+        wss.close();
+
+        process.nextTick(() => {
+          socket.end();
+        });
+      });
 
-      wss.on('close', done);
-      wss.close();
+      wss._server.on('close', () => {
+        serverCloseEventEmitted = true;
+      });
+
+      wss.on('close', () => {
+        assert.ok(serverCloseEventEmitted);
+        done();
+      });
+    });
+
+    it("emits the 'close' event if the server is already closed", (done) => {
+      const wss = new WebSocket.Server({ port: 0 }, () => {
+        wss.close(() => {
+          assert.strictEqual(wss._state, 2);
+          wss.on('close', done);
+          wss.close();
+        });
+      });
     });
   });