fix: prevent the socket from joining a room after disconnection

Calling `socket.join()` after disconnection would lead to a memory leak, because the room was never removed from the memory: ```js io.on("connection", (socket) => { socket.disconnect(); socket.join("room1"); // leak }); ``` Related: - #4067 - #4380 Backported from 18f3fda
socketio · Jun 26, 2022 · f223178 · f223178
1 parent 226cc16
commit f223178
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 19 deletions.
diff --git a/lib/namespace.js b/lib/namespace.js
@@ -163,25  163,31 @@ Namespace.prototype.add = function(client, query, fn){
   var self = this;
   this.run(socket, function(err){
     process.nextTick(function(){
-      if ('open' == client.conn.readyState) {
-        if (err) return socket.error(err.data || err.message);
-
-        // track socket
-        self.sockets[socket.id] = socket;
-
-        // it's paramount that the internal `onconnect` logic
-        // fires before user-set events to prevent state order
-        // violations (such as a disconnection before the connection
-        // logic is complete)
-        socket.onconnect();
-        if (fn) fn();
-
-        // fire user-set events
-        self.emit('connect', socket);
-        self.emit('connection', socket);
-      } else {
-        debug('next called after client was closed - ignoring socket');
       if ("open" !== client.conn.readyState) {
         debug("next called after client was closed - ignoring socket");
         socket._cleanup();
         return;
       }
 
       if (err) {
         debug("middleware error, sending CONNECT_ERROR packet to the client");
         socket._cleanup();
         return socket.error(err.data || err.message);
       }
 
       // track socket
       self.sockets[socket.id] = socket;
 
       // it's paramount that the internal `onconnect` logic
       // fires before user-set events to prevent state order
       // violations (such as a disconnection before the connection
       // logic is complete)
       socket.onconnect();
       if (fn) fn();
 
       // fire user-set events
       self.emit('connect', socket);
       self.emit('connection', socket);
     });
   });
   return socket;

diff --git a/lib/socket.js b/lib/socket.js
@@ -447,7  447,7 @@ Socket.prototype.onclose = function(reason){
   if (!this.connected) return this;
   debug('closing socket - reason %s', reason);
   this.emit('disconnecting', reason);
-  this.leaveAll();
   this._cleanup();
   this.nsp.remove(this);
   this.client.remove(this);
   this.connected = false;
@@ -576,3  576,8 @@ Socket.prototype.run = function(event, fn){
 
   run(0);
 };
 
 Socket.prototype._cleanup = function () {
   this.leaveAll();
   this.join = function noop() {};
 }
diff --git a/test/socket.io.js b/test/socket.io.js
@@ -1901,6  1901,43 @@ describe('socket.io', function(){
       });
     });
 
     it("should leave all rooms joined after a middleware failure", (done) => {
       const srv = http().listen(0);
       const sio = io(srv);
       const clientSocket = client(srv, "/");
 
       sio.use((socket, next) => {
         socket.join("room1");
         next(new Error("nope"));
       });
 
       clientSocket.on("error", () => {
         expect(sio.of("/").adapter.rooms).to.eql(0);
 
         clientSocket.disconnect();
         sio.close();
         done();
       });
     });
 
     it("should not join rooms after disconnection", (done) => {
       const srv = http().listen(0);
       const sio = io(srv);
       const clientSocket = client(srv, "/");
 
       sio.on("connection", (socket) => {
         socket.disconnect();
         socket.join("room1");
       });
 
       clientSocket.on("disconnect", () => {
         expect(sio.of("/").adapter.rooms).to.eql(0);
 
         sio.close();
         done();
       });
     });
 
     it('should always trigger the callback (if provided) when joining a room', function(done){
       var srv = http();
       var sio = io(srv);