From cb0140063e26eb56e937c2d4fae9d18216b8a80d Mon Sep 17 00:00:00 2001 From: Li Yuxuan Date: Fri, 13 Mar 2020 12:47:04 +0800 Subject: [PATCH] Fix goroutine leak when exec/attach The resize chan is never closed when doing exec/attach now. What's more, `resize` is a recieved only chan so it can not be closed. Use ctx to exit the goroutine in `handleResizing` properly. Signed-off-by: Li Yuxuan --- pkg/server/container_attach.go | 4 +++- pkg/server/container_execsync.go | 2 +- pkg/server/streaming.go | 22 +++++++++++++--------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/pkg/server/container_attach.go b/pkg/server/container_attach.go index 79aed7bb1..3497df3ad 100644 --- a/pkg/server/container_attach.go +++ b/pkg/server/container_attach.go @@ -44,6 +44,8 @@ func (c *criService) Attach(ctx context.Context, r *runtime.AttachRequest) (*run func (c *criService) attachContainer(ctx context.Context, id string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool, resize <-chan remotecommand.TerminalSize) error { + ctx, cancel := context.WithCancel(ctx) + defer cancel() // Get container from our container store. cntr, err := c.containerStore.Get(id) if err != nil { @@ -60,7 +62,7 @@ func (c *criService) attachContainer(ctx context.Context, id string, stdin io.Re if err != nil { return errors.Wrap(err, "failed to load task") } - handleResizing(resize, func(size remotecommand.TerminalSize) { + handleResizing(ctx, resize, func(size remotecommand.TerminalSize) { if err := task.Resize(ctx, uint32(size.Width), uint32(size.Height)); err != nil { log.G(ctx).WithError(err).Errorf("Failed to resize task %q console", id) } diff --git a/pkg/server/container_execsync.go b/pkg/server/container_execsync.go index d05ffd7d3..95a1c1d31 100644 --- a/pkg/server/container_execsync.go +++ b/pkg/server/container_execsync.go @@ -132,7 +132,7 @@ func (c *criService) execInternal(ctx context.Context, container containerd.Cont return nil, errors.Wrapf(err, "failed to start exec %q", execID) } - handleResizing(opts.resize, func(size remotecommand.TerminalSize) { + handleResizing(ctx, opts.resize, func(size remotecommand.TerminalSize) { if err := process.Resize(ctx, uint32(size.Width), uint32(size.Height)); err != nil { log.G(ctx).WithError(err).Errorf("Failed to resize process %q console for container %q", execID, id) } diff --git a/pkg/server/streaming.go b/pkg/server/streaming.go index 20ccd95fd..3063ac0f9 100644 --- a/pkg/server/streaming.go +++ b/pkg/server/streaming.go @@ -17,6 +17,7 @@ limitations under the License. package server import ( + "context" "crypto/tls" "io" "math" @@ -160,9 +161,8 @@ func (s *streamRuntime) PortForward(podSandboxID string, port int32, stream io.R } // handleResizing spawns a goroutine that processes the resize channel, calling resizeFunc for each -// remotecommand.TerminalSize received from the channel. The resize channel must be closed elsewhere to stop the -// goroutine. -func handleResizing(resize <-chan remotecommand.TerminalSize, resizeFunc func(size remotecommand.TerminalSize)) { +// remotecommand.TerminalSize received from the channel. +func handleResizing(ctx context.Context, resize <-chan remotecommand.TerminalSize, resizeFunc func(size remotecommand.TerminalSize)) { if resize == nil { return } @@ -171,14 +171,18 @@ func handleResizing(resize <-chan remotecommand.TerminalSize, resizeFunc func(si defer runtime.HandleCrash() for { - size, ok := <-resize - if !ok { + select { + case <-ctx.Done(): return + case size, ok := <-resize: + if !ok { + return + } + if size.Height < 1 || size.Width < 1 { + continue + } + resizeFunc(size) } - if size.Height < 1 || size.Width < 1 { - continue - } - resizeFunc(size) } }() }