Skip to content

fix: fix race conditions with dynamic namespaces (#4136) #4137

New issue

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

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

Already on GitHub? # to your account

Conversation

sebamarynissen
Copy link
Contributor

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behavior

A race condition exists when creating dynamic namespaces (see #4136)

New behavior

The race condition is fixed.

@sebamarynissen
Copy link
Contributor Author

I've changed the test a bit so that it does not rely on setTimeout(), hereby ensuring that the race condition is definitely present.

darrachequesne pushed a commit that referenced this pull request 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

Merged as 9d86397. Thanks!

@sebamarynissen
Copy link
Contributor Author

@darrachequesne Quick question: are you planning on releasing this as 4.3.2? I'm currently monkey-patching socket.io in my codebase, so it would be nice to be able to drop the patch!

@darrachequesne
Copy link
Member

Let's do this! https://github.com/socketio/socket.io/releases/tag/4.3.2

@darrachequesne darrachequesne added this to the 4.3.2 milestone Nov 8, 2021
dzad pushed a commit to dzad/socket.io that referenced this pull request 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants