From c2ba29c15b2174bac724472193a2604aac308a7d Mon Sep 17 00:00:00 2001 From: Jeremi Piotrowski Date: Fri, 25 Aug 2023 10:51:21 +0200 Subject: [PATCH] runtime: Fix data race in ioCopy IoCopy is a tricky function (I don't claim to fully understand its contract), but here is what I see: The goroutine that runs it spawns 3 goroutines - one for each stream to handle (stdin/stdout/stderr). The goroutine then waits for the stream goroutines to exit. The idea is that when the process exits and is closed, the stdout goroutine will be unblocked and close stdin - this should unblock the stdin goroutine. The stderr goroutine will exit at the same time as the stdout goroutine. The iocopy routine then closes all tty.io streams. The problem is that the stdout goroutine decrements the WaitGroup before closing the stdin stream, which causes the iocopy goroutine to race to close the streams. Move the wg.Done() of the stdout routine past the close so that *this* race becomes impossible. I can't guarantee that this doesn't affect some unspecified behavior. Fixes: #5031 Signed-off-by: Jeremi Piotrowski --- src/runtime/pkg/containerd-shim-v2/stream.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/pkg/containerd-shim-v2/stream.go b/src/runtime/pkg/containerd-shim-v2/stream.go index c20e63de82..123c1c1148 100644 --- a/src/runtime/pkg/containerd-shim-v2/stream.go +++ b/src/runtime/pkg/containerd-shim-v2/stream.go @@ -126,11 +126,11 @@ func ioCopy(shimLog *logrus.Entry, exitch, stdinCloser chan struct{}, tty *ttyIO p := bufPool.Get().(*[]byte) defer bufPool.Put(p) io.CopyBuffer(tty.io.Stdout(), stdoutPipe, *p) - wg.Done() if tty.io.Stdin() != nil { // close stdin to make the other routine stop tty.io.Stdin().Close() } + wg.Done() shimLog.Debug("stdout io stream copy exited") }() }