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

docs: clarify the semantics of concurrent Read calls #136

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

tgross
Copy link
Member

@tgross tgross commented Oct 1, 2024

Concurrent operations on the stream are "safe" inasmuch as they don't cause a data race. That is, calling close concurrently with a read won't panic, you can write and read concurrently, and reading from two different goroutines shouldn't result in short reads. But there's no intended guarantee that the reads are semantically meaningful if you're reading concurrently.

Fixes: #128
Fixes: https://hashicorp.atlassian.net/browse/NET-10293

Concurrent operations on the stream are "safe" inasmuch as they don't cause a
data race. That is, calling close concurrently with a read won't panic, you can
write and read concurrently, and reading from two different goroutines shouldn't
result in short reads. But there's no intended guarantee that the reads are
semantically meaningful if you're reading concurrently.

Fixes: #128
@tgross tgross requested review from rboyer and schmichael October 1, 2024 12:58
@tgross tgross merged commit 49eecf3 into master Oct 2, 2024
3 checks passed
tgross added a commit that referenced this pull request Dec 3, 2024
In #136 we documented warnings around the semantics of calling `Stream.Read`
concurrently. But we assumed without investigating that the concurrent
operations were actually safe. But in fact the locks made by `Read` and `Close`
calls can interleave, causing a deadlock. Update the documentation again to make
this clear.

Fixes: #139
tgross added a commit that referenced this pull request Dec 3, 2024
In #136 we documented warnings around the semantics of calling `Stream.Read`
concurrently. But we assumed without investigating that the concurrent
operations were actually safe. But in fact the locks made by multiple Read calls
can interleave, causing a deadlock. Update the documentation again to make this
clear.

Fixes: #139
tgross added a commit that referenced this pull request Dec 4, 2024
In #136 we documented warnings around the semantics of calling `Stream.Read`
concurrently. But we assumed without investigating that the concurrent
operations were actually safe. But in fact the locks made by multiple Read calls
can interleave, causing a deadlock. Update the documentation again to make this
clear.

Fixes: #139
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.

Concurrent calls to Stream.Read can return different data
2 participants