From: wheat2018 <1151937289@qq.com> Date: Tue, 13 Aug 2024 15:56:31 +0800 Subject: fix goroutine leak of container Attach The monitor goroutine (runs (*ContainerIO).Attach.func1) of Attach will never finish if it attaches to a container without any stdout or stderr output. Wait for http context cancel and break the pipe actively to address the issue. Signed-off-by: wheat2018 <1151937289@qq.com> Signed-off-by: Akihiro Suda (cherry picked from commit a0d0f0ef68935338d2c710db164fa7820f692530) Signed-off-by: Akihiro Suda --- pkg/cri/io/container_io.go | 14 +++++++++++--- pkg/cri/sbserver/container_attach.go | 2 +- pkg/cri/server/container_attach.go | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/cri/io/container_io.go b/pkg/cri/io/container_io.go index 70bc8b7..e158410 100644 --- a/pkg/cri/io/container_io.go +++ b/pkg/cri/io/container_io.go @@ -17,6 +17,7 @@ package io import ( + "context" "errors" "io" "strings" @@ -134,7 +135,7 @@ func (c *ContainerIO) Pipe() { // Attach attaches container stdio. // TODO(random-liu): Use pools.Copy in docker to reduce memory usage? -func (c *ContainerIO) Attach(opts AttachOptions) { +func (c *ContainerIO) Attach(ctx context.Context, opts AttachOptions) { var wg sync.WaitGroup key := util.GenerateID() stdinKey := streamKey(c.id, "attach-"+key, Stdin) @@ -175,8 +176,15 @@ func (c *ContainerIO) Attach(opts AttachOptions) { } attachStream := func(key string, close <-chan struct{}) { - <-close - logrus.Infof("Attach stream %q closed", key) + select { + case <-close: + logrus.Infof("Attach stream %q closed", key) + case <-ctx.Done(): + logrus.Infof("Attach client of %q cancelled", key) + // Avoid writeGroup heap up + c.stdoutGroup.Remove(key) + c.stderrGroup.Remove(key) + } // Make sure stdin gets closed. if stdinStreamRC != nil { stdinStreamRC.Close() diff --git a/pkg/cri/sbserver/container_attach.go b/pkg/cri/sbserver/container_attach.go index 8527071..315ac8a 100644 --- a/pkg/cri/sbserver/container_attach.go +++ b/pkg/cri/sbserver/container_attach.go @@ -79,6 +79,6 @@ func (c *criService) attachContainer(ctx context.Context, id string, stdin io.Re }, } // TODO(random-liu): Figure out whether we need to support historical output. - cntr.IO.Attach(opts) + cntr.IO.Attach(ctx, opts) return nil } diff --git a/pkg/cri/server/container_attach.go b/pkg/cri/server/container_attach.go index d7d0096..465b3d8 100644 --- a/pkg/cri/server/container_attach.go +++ b/pkg/cri/server/container_attach.go @@ -79,6 +79,6 @@ func (c *criService) attachContainer(ctx context.Context, id string, stdin io.Re }, } // TODO(random-liu): Figure out whether we need to support historical output. - cntr.IO.Attach(opts) + cntr.IO.Attach(ctx, opts) return nil }