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

fix(ext/websocket): Avoid write deadlock that requires read_frame to complete #18705

Merged
merged 3 commits into from Apr 14, 2023

Conversation

littledivy
Copy link
Member

Fixes #18700

Timeline of the events that lead to the bug.

  1. WebSocket handshake complete
  2. Server on read_frame holding an AsyncRefCell borrow of the WebSocket stream.
  3. Client sends a TXT frame after a some time
  4. Server recieves the frame and goes back to read_frame.
  5. After some time, Server starts a write_frame but read_frame is still holding a borrow!
    ^--- Locked. read_frame needs to complete so we can resume the write.

This commit changes all writes to directly borrow the fastwebsocket::WebSocket resource under the assumption that it won't affect ongoing reads.

@littledivy littledivy merged commit a411144 into denoland:main Apr 14, 2023
11 checks passed
levex pushed a commit that referenced this pull request Apr 18, 2023
…complete (#18705)

Fixes #18700

Timeline of the events that lead to the bug.

1. WebSocket handshake complete
2. Server on `read_frame` holding an AsyncRefCell borrow of the
WebSocket stream.
3. Client sends a TXT frame after a some time
4. Server recieves the frame and goes back to `read_frame`.
5. After some time, Server starts a `write_frame` but `read_frame` is
still holding a borrow!
^--- Locked. read_frame needs to complete so we can resume the write.

This commit changes all writes to directly borrow the
`fastwebsocket::WebSocket` resource under the assumption that it won't
affect ongoing reads.
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.

Upgraded WebSockets Don't Flush Frames @ v1.32.4
2 participants