Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition with dynamic namespaces #4136

Closed
sebamarynissen opened this issue Oct 22, 2021 · 2 comments
Closed

Race condition with dynamic namespaces #4136

sebamarynissen opened this issue Oct 22, 2021 · 2 comments
Labels
bug Something isn't working
Milestone

Comments

@sebamarynissen
Copy link
Contributor

I have noticed that a race condition exists with dynamic namespaces. Consider the following setup

// Server side
import { Server } from 'socket.io';
const io = new Server(3000, {});

io.of(async (nsp, auth, next) => {
  let exists = !!await queryDatabaseForResource({});
  next(null, exists);
});

If the query to the database takes some time and a lot of clients are trying to connect to the same namespace in a short period of time, the namespace will be created multiple times. This will result in sockets being connected to a namespace that is overridden by another one, and hence they will no longer be receiving events.

Looking at Server.prototype._checkNamespace, I think this can be solved as follows:

Server.prototype._checkNamespace = function(name, auth, fn) {
  if (this.parentNsps.size === 0) return fn(false);
  const keysIterator = this.parentNsps.keys();
  const run = () => {
    const nextFn = keysIterator.next();
    if (nextFn.done) return fn(false);
    nextFn.value(name, auth, (err, allow) => {
      if (err || !allow) {
        run();
      } else if (this._nsps.has(name)) {

        // If the namespace has been created in the meantime, do not create it again.
        return fn(this._nsps.get(name));

      } else {

        const namespace = this.parentNsps
          .get(nextFn.value)
          .createChild(name);

        this.sockets.emitReserved('new_namespace', namespace);
        fn(namespace);

      }
    });
  };
  run();
};

I will file a PR to solve this.

Context

The odds of this race condition happening are obviously rather low, but I experienced them on a website of mine where I organize tournaments. If the tournament starts, people are all sent to a socket.io namespace where their match is hosted. These namespaces are created dynamically, and so it happens that a large amount of people tries to connect at the same moment.

@sebamarynissen
Copy link
Contributor Author

PR filed (#4137).

darrachequesne pushed a commit that referenced this issue Oct 24, 2021
Using an async operation with `io.use()` could lead to the creation of
several instances of a same namespace, each of them overriding the
previous one.

Example:

```js
io.use(async (nsp, auth, next) => {
  await anOperationThatTakesSomeTime();
  next();
});
```

Related: #4136
@darrachequesne
Copy link
Member

Good catch! Thanks for the detailed analysis 👍

This should be fixed by 9d86397

@darrachequesne darrachequesne added this to the 4.3.2 milestone Nov 8, 2021
darrachequesne added a commit that referenced this issue Jun 26, 2022
Using an async operation with `io.use()` could lead to the creation of
several instances of a same namespace, each of them overriding the
previous one.

Example:

```js
io.use(async (nsp, auth, next) => {
  await anOperationThatTakesSomeTime();
  next();
});
```

Related: #4136

Backported from 9d86397
dzad pushed a commit to dzad/socket.io that referenced this issue May 29, 2023
Using an async operation with `io.use()` could lead to the creation of
several instances of a same namespace, each of them overriding the
previous one.

Example:

```js
io.use(async (nsp, auth, next) => {
  await anOperationThatTakesSomeTime();
  next();
});
```

Related: socketio#4136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants