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
  • Loading branch information
darrachequesne committed May 25, 2022
1 parent 5ab8289 commit 18f3fda
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 26 deletions.
52 changes: 28 additions & 24 deletions lib/namespace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,34 +218,38 @@ export class Namespace<
const socket = new Socket(this, client, query);
this.run(socket, (err) => {
process.nextTick(() => {
if ("open" == client.conn.readyState) {
if (err) {
if (client.conn.protocol === 3) {
return socket._error(err.data || err.message);
} else {
return socket._error({
message: err.message,
data: err.data,
});
}
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();
if (client.conn.protocol === 3) {
return socket._error(err.data || err.message);
} else {
return socket._error({
message: err.message,
data: err.data,
});
}
}

// track socket
this.sockets.set(socket.id, socket);
// track socket
this.sockets.set(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();
// 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
this.emitReserved("connect", socket);
this.emitReserved("connection", socket);
} else {
debug("next called after client was closed - ignoring socket");
}
// fire user-set events
this.emitReserved("connect", socket);
this.emitReserved("connection", socket);
});
});
return socket;
Expand Down
14 changes: 13 additions & 1 deletion lib/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ export interface Handshake {
*/
export type Event = [string, ...any[]];

function noop() {}

export class Socket<
ListenEvents extends EventsMap = DefaultEventsMap,
EmitEvents extends EventsMap = ListenEvents,
Expand Down Expand Up @@ -511,14 +513,24 @@ export class Socket<
if (!this.connected) return this;
debug("closing socket - reason %s", reason);
this.emitReserved("disconnecting", reason);
this.leaveAll();
this._cleanup();
this.nsp._remove(this);
this.client._remove(this);
this.connected = false;
this.emitReserved("disconnect", reason);
return;
}

/**
* Makes the socket leave all the rooms it was part of and prevents it from joining any other room
*
* @private
*/
_cleanup() {
this.leaveAll();
this.join = noop;
}

/**
* Produces an `error` packet.
*
Expand Down
37 changes: 36 additions & 1 deletion test/socket.io.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";

import { Server, Socket, Namespace } from "../lib";
import { Server, Socket, Namespace } from "..";
import { createServer } from "http";
import fs = require("fs");
import { join } from "path";
Expand All @@ -11,6 +11,7 @@ import type { AddressInfo } from "net";
import * as io_v2 from "socket.io-client-v2";
import type { SocketId } from "socket.io-adapter";
import { io as ioc, Socket as ClientSocket } from "socket.io-client";
import { createClient } from "./support/util";

import "./support/util";
import "./utility-methods";
Expand Down Expand Up @@ -2109,6 +2110,40 @@ describe("socket.io", () => {
});
});

it("should leave all rooms joined after a middleware failure", (done) => {
const io = new Server(0);
const client = createClient(io, "/");

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

client.on("connect_error", () => {
expect(io.of("/").adapter.rooms.size).to.eql(0);

io.close();
done();
});
});

it("should not join rooms after disconnection", (done) => {
const io = new Server(0);
const client = createClient(io, "/");

io.on("connection", (socket) => {
socket.disconnect();
socket.join("room1");
});

client.on("disconnect", () => {
expect(io.of("/").adapter.rooms.size).to.eql(0);

io.close();
done();
});
});

describe("onAny", () => {
it("should call listener", (done) => {
const srv = createServer();
Expand Down

0 comments on commit 18f3fda

Please sign in to comment.