Skip to content

Commit

Permalink
fix: prevent the socket from joining a room after disconnection
Browse files Browse the repository at this point in the history
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
  • Loading branch information
darrachequesne committed Jun 26, 2022
1 parent 226cc16 commit f223178
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 19 deletions.
42 changes: 24 additions & 18 deletions lib/namespace.js
Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion lib/socket.js
Expand Up @@ -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;
Expand Down Expand Up @@ -576,3 +576,8 @@ Socket.prototype.run = function(event, fn){

run(0);
};

Socket.prototype._cleanup = function () {
this.leaveAll();
this.join = function noop() {};
}
37 changes: 37 additions & 0 deletions test/socket.io.js
Expand Up @@ -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);
Expand Down

0 comments on commit f223178

Please sign in to comment.