From 04745a7cc38eca09ff9e9e3ccd91044038e0841d Mon Sep 17 00:00:00 2001 From: Erik Sipsma Date: Fri, 3 May 2024 19:40:17 -0700 Subject: [PATCH] progress: fix leak of pipe goroutine from MultiReader 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 --- util/progress/multireader.go | 1 + 1 file changed, 1 insertion(+) diff --git a/util/progress/multireader.go b/util/progress/multireader.go index d6d3fb7c79a9..6afcca9feb3a 100644 --- a/util/progress/multireader.go +++ b/util/progress/multireader.go @@ -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 {