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

[disk] drain any remaining data before Put returns #474

Merged
merged 1 commit into from
Sep 22, 2021

Conversation

mostynb
Copy link
Collaborator

@mostynb mostynb commented Sep 20, 2021

io.PipeWriter Write calls block until all the data written in the call is read from the corresponding io.PipeReader. If we don't read all that data, then the writing goroutine will block forever.

The PipeWriter used in bytestream Write calls is intended to be consumed by disk.Put(), but if that returns early then there will be blocked writes. To un-block them, we should ensure that any remaining data is drained before disk.Put returns.

This might fix #473

@mostynb mostynb force-pushed the fix_goroutine_leak branch 2 times, most recently from 911127a to ef5e162 Compare September 20, 2021 20:32
@mostynb mostynb changed the title [bytestream] drain any remaining data after Put returns [disk] drain any remaining data before Put returns Sep 20, 2021
io.PipeWriter Write calls block until all the data written in the
call is read from the corresponding io.PipeReader. If we don't read
all that data, then the writing goroutine will block forever.

The PipeWriter used in bytestream Write calls is intended to be
consumed by disk.Put(), but if that returns early then there will
be blocked writes. To un-block them, we should ensure that any
remaining data is drained before disk.Put returns.

This might fix buchgr#473
@mostynb mostynb merged commit a914031 into buchgr:master Sep 22, 2021
@mostynb mostynb deleted the fix_goroutine_leak branch September 22, 2021 20:02
mostynb added a commit to mostynb/bazel-remote that referenced this pull request Sep 23, 2021
io.PipeWriter Write calls block until all the data written in the
call is read from the corresponding io.PipeReader. If we don't read
all that data, then the writing goroutine will block forever.

The PipeWriter used in bytestream Write calls is intended to be
consumed by disk.Put(), but if that returns early then there will
be blocked writes. To un-block them, we should ensure that any
remaining data is drained before disk.Put returns.

Backport of buchgr#474
mostynb added a commit to mostynb/bazel-remote that referenced this pull request Sep 23, 2021
io.PipeWriter Write calls block until all the data written in the
call is read from the corresponding io.PipeReader. If we don't read
all that data, then the writing goroutine will block forever.

The PipeWriter used in bytestream Write calls is intended to be
consumed by disk.Put(), but if that returns early then there will
be blocked writes. To un-block them, we should ensure that any
remaining data is drained before disk.Put returns.

Backport of buchgr#474
mostynb added a commit that referenced this pull request Sep 23, 2021
io.PipeWriter Write calls block until all the data written in the
call is read from the corresponding io.PipeReader. If we don't read
all that data, then the writing goroutine will block forever.

The PipeWriter used in bytestream Write calls is intended to be
consumed by disk.Put(), but if that returns early then there will
be blocked writes. To un-block them, we should ensure that any
remaining data is drained before disk.Put returns.

Backport of #474
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.

bytestream write can leak goroutines if disk.Put doesn't drain the io.Reader
1 participant