Skip to content

Commit

Permalink
progress: fix leak of pipe goroutine from MultiReader
Browse files Browse the repository at this point in the history
In the case where the context provided to MultiReader.Reader is
cancelled, the writer will be deleted.

Before this, closeWriter was not called in that codepath, when meant
that MultiReader.handle would never see the writer and never close it.
* I don't have an isolated repro, but when running a few hundred of
  Dagger's tests I was seeing several thousand goroutines blocked on
  waiting for the progress pipe's context to be done.

Now, we just call closeWriter when it gets deleted so that the pipe's
goroutine doesn't leak.
* After this change, those thousands of leaked goroutines are entirely
  gone.

Signed-off-by: Erik Sipsma <[email protected]>
  • Loading branch information
sipsma committed May 4, 2024
1 parent 51d85d7 commit 04745a7
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions util/progress/multireader.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func (mr *MultiReader) Reader(ctx context.Context) Reader {
mr.mu.Lock()
defer mr.mu.Unlock()
delete(mr.writers, w)
closeWriter(context.Cause(ctx))
}()

if !mr.initialized {
Expand Down

0 comments on commit 04745a7

Please sign in to comment.