Merge commit from fork

dmcgowan · web-flow · commit 083b53cd6f19 · 2025-11-05T13:49:35.000-08:00
[Carry #1] fix goroutine leak of container Attach
diff --git a/internal/cri/io/container_io.go b/internal/cri/io/container_io.go
@@ -17,6 +17,7 @@
 package io
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -160,7 +161,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)
@@ -201,8 +202,15 @@ func (c *ContainerIO) Attach(opts AttachOptions) {
 	}
 
 	attachStream := func(key string, close <-chan struct{}) {
-		<-close
-		log.L.Infof("Attach stream %q closed", key)
+		select {
+		case <-close:
+			log.L.Infof("Attach stream %q closed", key)
+		case <-ctx.Done():
+			log.L.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/internal/cri/server/container_attach.go b/internal/cri/server/container_attach.go
@@ -82,6 +82,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
 }

Original file line number	Diff line number	Diff line change
`@@ -82,6 +82,6 @@ func (c *criService) attachContainer(ctx context.Context, id string, stdin io.Re`
`82`	`82`	`},`
`83`	`83`	`}`
`84`	`84`	`// TODO(random-liu): Figure out whether we need to support historical output.`
`85`		`- cntr.IO.Attach(opts)`
	`85`	`+ cntr.IO.Attach(ctx, opts)`
`86`	`86`	`return nil`
`87`	`87`	`}`