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

Why does Stream.Read notify recvNotifyCh? #132

Open
schmichael opened this issue Aug 16, 2024 · 0 comments
Open

Why does Stream.Read notify recvNotifyCh? #132

schmichael opened this issue Aug 16, 2024 · 0 comments

Comments

@schmichael
Copy link
Member

I want to record a subset of @lattwood's investigation from #127 here:

  • Stream.recvNotifyCh is ticked asynchronously after Stream.Read returns, through defer asyncNotify(s.recvNotifyCh)
    • yamux Streams are not threadsafe (see Concurrent calls to Stream.Read can return different data #128), so there is no reason that should ever wake anything up, because there should only be one call to Read at a time anyways. (and I don't think that go guarantees the first goroutine to block on a channel is the first one do get a message)
  • this is the one that makes me think there's more synchronization bugs in the code :(

I agree with their assessment. I can't see how this code could cause bugs, but as OP's last bullet indicates: it sure makes it seem like it was added to work around other bugs.

Recording here as it warrants more investigation to either remove, or comment with a reason.

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

No branches or pull requests

1 participant