Open
Description
I want to record a subset of @lattwood's investigation from #127 here:
Stream.recvNotifyCh
is ticked asynchronously afterStream.Read
returns, throughdefer 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)
- they're not threadsafe because the Read call that gets woken up is non-deterministic. See example here: https://go.dev/play/p/4QR3HvLoGyz
- 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.
Metadata
Assignees
Labels
No labels