Skip to content

Conversation

@mmastrac
Copy link
Contributor

@mmastrac mmastrac commented Jan 27, 2025

Ensure that wakers are registered in a thread-safe manner.

This is a latent multi-threaded bug that could happen, though it's a very, very tight race to win.

In a multi-threaded runtime, there was a very small window for a race:

  1. The send_handshake part of the handshake task completes, and the handshake then tries to wake any deferred wakers (which do not exist). The task is about to complete but then the thread gets parked by the CPU.
  2. In another thread, a poll_read operation (or similar) tests to see if the handshake finished, sees that it isn't and tries to register its waker. The handshake task has already tried to wake the deferred wakers, so nothing ever tries to wake them again.
  3. The handshake task completes, but no wakers are woken to complete the handshake process, locking up the connection.

This wasn't easy to repro on my Mac -- I had to explicitly add a thread sleep after the poll_read check for handle.is_finished(), but before we set the waker. This would trigger the race pretty much every time.

@mmastrac mmastrac mentioned this pull request Jan 27, 2025
@bartlomieju bartlomieju merged commit 415d580 into denoland:main Jan 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants