From 38ff4f8fd45e4f9124154e9bcfaaa9577b64c74f Mon Sep 17 00:00:00 2001 From: Sebastiaan Marynissen Date: Fri, 22 Oct 2021 19:09:00 +0200 Subject: [PATCH 1/2] fix: fix race conditions with dynamic namespaces (#4136) --- lib/client.ts | 9 +++++++-- lib/index.ts | 7 ++++++- test/socket.io.ts | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/lib/client.ts b/lib/client.ts index 268cfbd085..8dc02c1d02 100644 --- a/lib/client.ts +++ b/lib/client.ts @@ -113,10 +113,15 @@ export class Client< ( dynamicNspName: | Namespace - | false + | false, + race: boolean = false ) => { if (dynamicNspName) { - debug("dynamic namespace %s was created", dynamicNspName); + if (race) { + debug("dynamic namespace %s already created", dynamicNspName); + } else { + debug("dynamic namespace %s was created", dynamicNspName); + } this.doConnect(name, auth); } else { debug("creation of namespace %s was denied", name); diff --git a/lib/index.ts b/lib/index.ts index 72c87ef677..d4505cc32a 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -188,7 +188,8 @@ export class Server< name: string, auth: { [key: string]: any }, fn: ( - nsp: Namespace | false + nsp: Namespace | false, + race?: boolean ) => void ): void { if (this.parentNsps.size === 0) return fn(false); @@ -203,6 +204,10 @@ export class Server< nextFn.value(name, auth, (err, allow) => { if (err || !allow) { run(); + } else if (this._nsps.has(name)) { + // See #4316. It's possible that in the meantime the namespace has + // already been created, so we'll have to handle this properly. + fn(this._nsps.get(name) as Namespace, true); } else { const namespace = this.parentNsps .get(nextFn.value)! diff --git a/test/socket.io.ts b/test/socket.io.ts index 4777796f89..74efb0e13a 100644 --- a/test/socket.io.ts +++ b/test/socket.io.ts @@ -998,6 +998,42 @@ describe("socket.io", () => { const socket = client(srv, "/dynamic-101"); }); }); + + it("should handle race conditions with dynamic namespaces (#4136)", (done) => { + const srv = createServer(); + const sio = new Server(srv); + const counters = { + connected: 0, + created: 0, + events: 0, + }; + sio.on("new_namespace", (namespace) => { + counters.created++; + }); + srv.listen(() => { + const handler = () => { + if (++counters.events === 2) { + expect(counters.created).to.equal(1); + done(); + } + }; + + sio + .of((name, query, next) => { + setTimeout(() => next(null, true), 50); + }) + .on("connection", (socket) => { + if (++counters.connected === 2) { + sio.of("/dynamic-101").emit("message"); + } + }); + + let one = client(srv, "/dynamic-101"); + let two = client(srv, "/dynamic-101"); + one.on("message", handler); + two.on("message", handler); + }); + }); }); }); From d09de3d99549cf26713918c999c3a56a6b41983e Mon Sep 17 00:00:00 2001 From: Sebastiaan Marynissen Date: Sat, 23 Oct 2021 16:52:48 +0200 Subject: [PATCH 2/2] fix: avoid relying on `setTimeout()` in tests --- lib/index.ts | 2 +- test/socket.io.ts | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/index.ts b/lib/index.ts index d4505cc32a..f38819195f 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -205,7 +205,7 @@ export class Server< if (err || !allow) { run(); } else if (this._nsps.has(name)) { - // See #4316. It's possible that in the meantime the namespace has + // See #4136. It's possible that in the meantime the namespace has // already been created, so we'll have to handle this properly. fn(this._nsps.get(name) as Namespace, true); } else { diff --git a/test/socket.io.ts b/test/socket.io.ts index 74efb0e13a..63e7f1b830 100644 --- a/test/socket.io.ts +++ b/test/socket.io.ts @@ -15,6 +15,8 @@ import { io as ioc, Socket as ClientSocket } from "socket.io-client"; import "./support/util"; import "./utility-methods"; +type callback = (err: Error | null, success: boolean) => void; + // Creates a socket.io client for the given server function client(srv, nsp?: string | object, opts?: object): ClientSocket { if ("object" == typeof nsp) { @@ -1007,6 +1009,7 @@ describe("socket.io", () => { created: 0, events: 0, }; + const buffer: callback[] = []; sio.on("new_namespace", (namespace) => { counters.created++; }); @@ -1020,7 +1023,10 @@ describe("socket.io", () => { sio .of((name, query, next) => { - setTimeout(() => next(null, true), 50); + buffer.push(next); + if (buffer.length === 2) { + buffer.forEach((next) => next(null, true)); + } }) .on("connection", (socket) => { if (++counters.connected === 2) {