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 <jpiotrowski@microsoft.com>
This commit is contained in:
Jeremi Piotrowski 2023-08-25 10:51:21 +02:00
parent b467f2ef68
commit c2ba29c15b

View File

@ -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")
}()
}