socket.join could lead to a room leak.

[baptistejamin]

Describe the bug

We are using socket io since years now, and we discovered a very weird bug that can occur under some conditions.

It seems it's possible to join rooms using the base adapter, even if the socket is destroyed or disconnected. I don't know if it was intended initially, but adapter.addAll shouldn't be connected if the socket is no longer available.

A such issue could appear for instance under an elevated database latency. If the client socket disconnected before the socket.join action, the join action is performed anyway.

To Reproduce

1. Use the client
2. Leave the page before 15s

Run those steps 5 times.

DUMMY_ROOM should have 5 different sockets attached to the rooms, even if those sockets are no longer connected.

Please fill the following code example:

Socket.IO server version: any version

Server

import { Server } from "socket.io";

const io = new Server(3000, {});

io.on("connection", (socket) => {
  socket.on("ping", () => {
    // Simulate a database latency
    new Promise((resolve) => {
      setTimeout(resolve, 15000);
    })
    .then(() => {
      socket.join("DUMMY_ROOM");
    });
  })

  socket.on("disconnect", () => {
    console.log(`disconnect ${socket.id}`);
  });
});

Socket.IO client version: x.y.z

Client

import { io } from "socket.io-client";

const socket = io("ws://localhost:3000/", {});

socket.on("connect", () => {
  socket.emit("ping")
});

socket.on("disconnect", () => {
  console.log("disconnect");
});

Expected behavior

socket.join shouldn't propagate this.adapter.addAll if the socket no longer exists.

[darrachequesne]

I could indeed reproduce, thanks :+1:

It can also happen within a middleware:

io.use((socket, next) => {
  socket.join("DUMMY_ROOM");
  next(new Error("nope"));

  console.log(io.of("/").adapter.rooms); Map(1) { 'DUMMY_ROOM' => Set(1) { 'pqUiVlJ7Q6juccjiAAAB' } }
});

This could explain issues like https://github.com/socketio/socket.io/issues/4067

Let's fix that.

[darrachequesne]

OK, so this should be fixed by https://github.com/socketio/socket.io/commit/18f3fdab12947a9fee3e9c37cfc1da97027d1473 I think.

[HsinHeng]

Thanks for to fix it. Does it plan to release at next version ?

[jiouiuw]

@HsinHeng released.

this issue should be closed

[HsinHeng]

@HsinHeng released.

this issue should be closed

Thanks for your notified.

[darrachequesne]

This should indeed be fixed by https://github.com/socketio/socket.io/commit/18f3fdab12947a9fee3e9c37cfc1da97027d1473, included in version 4.5.2 (and backported in the 2.x branch: https://github.com/socketio/socket.io/commit/f223178eb655a7713303b21a78f9ef9e161d6458).

Thanks for the clear and detailed report :heart:

Please reopen if needed.