Skip to content

Fix yamux stream handle split#424

Open
driftluo wants to merge 2 commits intomasterfrom
fix-yamux-stream-handle-split
Open

Fix yamux stream handle split#424
driftluo wants to merge 2 commits intomasterfrom
fix-yamux-stream-handle-split

Conversation

@driftluo
Copy link
Collaborator

@driftluo driftluo commented Mar 4, 2026

When self.recv_frames_wake(cx) is called on the write side, it passes the write side's context/waker to the channel. This overwrites the read side's waker, preventing the message from waking up and causing messages to be unreadable. This PR fixes this overwriting error and provides several test cases to ensure correct behavior.

Just so you know, this problem only manifests when the StreamHandle is split; when it's not split, reading and writing share a single waker without issue.

@driftluo driftluo force-pushed the fix-yamux-stream-handle-split branch 5 times, most recently from 62b2f8f to dffee87 Compare March 4, 2026 12:57
@driftluo driftluo force-pushed the fix-yamux-stream-handle-split branch from dffee87 to 8622684 Compare March 4, 2026 13:19
@driftluo driftluo marked this pull request as ready for review March 4, 2026 13:39
@driftluo driftluo requested a review from eval-exec as a code owner March 4, 2026 13:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a deadlock in yamux::StreamHandle when the handle is split into read/write halves: the write side previously polled frame_receiver with the write task’s waker, overwriting the read side’s waker and preventing reads from being awakened.

Changes:

  • Refactors frame draining on the write side to use a non-waker-registering path (try_recv_frames) to avoid overwriting the read task’s waker.
  • Adds deterministic regression tests in yamux/src/stream.rs validating correct waker behavior across split read/write polling.
  • Adds tentacle integration tests (behind unstable) exercising first-message and multi-message spawn flows; bumps socket2 dependency.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
yamux/src/stream.rs Introduces try_recv_frames and adjusts waker registration/wake-up behavior; adds multiple regression tests.
tentacle/tests/test_spawn_first_message.rs Adds integration tests covering the spawn-based first message and sustained bidirectional messaging paths.
tentacle/Cargo.toml Updates socket2 from 0.5.0 to 0.6.0.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

3 participants