From e7a9d25713d8568a104c23a966445c626d6e09d7 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Thu, 18 Nov 2021 11:39:14 +0100 Subject: [PATCH] fix: fix `socket.disconnect().connect()` usage Previously, calling `socket.disconnect().connect()` could, if the connection was upgraded to WebSocket, result in "disconnect" being emitted twice, and an engine being leaked. Here's what happened: > socket.disconnect() - calls `socket.destroy()` so the socket doesn't listen to the manager events anymore - then calls `manager._close()` which closes the underlying engine but not the manager itself (it waits for the "close" event of the engine) > socket.connect() - calls `socket.subEvents()` so the socket does listen to the manager events - calls `manager.open()` which creates a new engine And then the first engine emits a "close" event, which is forwarded to the socket, hence the second "disconnect" event. Related: https://github.com/socketio/socket.io-client/issues/1014 Backported from https://github.com/socketio/socket.io-client/commit/99c2cb8421361487ed7c876edd8670bb69a5c5b5 --- lib/manager.js | 10 ++-------- test/socket.js | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/lib/manager.js b/lib/manager.js index 8fb640305..3c92e36ad 100644 --- a/lib/manager.js +++ b/lib/manager.js @@ -481,13 +481,7 @@ Manager.prototype.disconnect = function () { debug('disconnect'); this.skipReconnect = true; this.reconnecting = false; - if ('opening' === this.readyState) { - // `onclose` will not fire because - // an open event never happened - this.cleanup(); - } - this.backoff.reset(); - this.readyState = 'closed'; + this.onclose('forced close'); if (this.engine) this.engine.close(); }; @@ -498,7 +492,7 @@ Manager.prototype.disconnect = function () { */ Manager.prototype.onclose = function (reason) { - debug('onclose'); + debug('closed due to %s', reason); this.cleanup(); this.backoff.reset(); diff --git a/test/socket.js b/test/socket.js index 91abd9d81..e3a82f8eb 100644 --- a/test/socket.js +++ b/test/socket.js @@ -176,4 +176,24 @@ describe('socket', function () { done(); }); }); + + it('should properly disconnect then reconnect', (done) => { + const socket = io('/', { forceNew: true, transports: ['websocket'] }); + + let count = 0; + + socket.once('connect', () => { + socket.disconnect().connect(); + }); + + socket.on('disconnect', () => { + count++; + }); + + setTimeout(() => { + expect(count).to.eql(1); + socket.disconnect(); + done(); + }, 200); + }); });