Skip to content

Commit

Permalink
Avoid a goroutine leak by ensuring pipes closed
Browse files Browse the repository at this point in the history
The func runCmd uses io.Pipe for both stderr and stdout of the command
it runs, so writes on either can be echoed to the log as they happen;
and two goroutines for doing that echoing. In process dumps, it's
apparent that these goroutines leak -- they are blocked on Scan, which
is presumably waiting for an EOF.

This commit replaces the explicit pipe plumbing with
`Command.StdoutPipe()` and `Command.StderrPipe()`. The Command machinery
knows to close the readers when the process has finished writing, so
Scan no longer blocks indefinitely.

It's also not necessary to start _two_ new goroutines, since we already
have one -- that in which we're running.

Signed-off-by: Michael Bridgen <[email protected]>
  • Loading branch information
squaremo committed Sep 13, 2022
1 parent ac24afc commit 8f69a7d
Showing 1 changed file with 23 additions and 17 deletions.
40 changes: 23 additions & 17 deletions pkg/controller/stack/stack_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -620,38 +619,45 @@ func (sess *reconcileStackSession) runCmd(title string, cmd *exec.Cmd, workspace
}

// Capture stdout and stderr.
stdoutR, stdoutW := io.Pipe()
stderrR, stderrW := io.Pipe()
cmd.Stdout = stdoutW
cmd.Stderr = stderrW
stdoutR, err := cmd.StdoutPipe()
if err != nil {
return "", "", err
}
stderrR, err := cmd.StderrPipe()
if err != nil {
return "", "", err
}

// Start the command asynchronously.
err := cmd.Start()
err = cmd.Start()
if err != nil {
return "", "", err
}

// Kick off some goroutines to stream the output asynchronously. Since Pulumi can take
// a while to run, this helps to debug issues that might be ongoing before a command completes.
var stdout bytes.Buffer
var stderr bytes.Buffer

// We want to echo both stderr and stdout as they are written; so at least one of them must be
// in another goroutine.
stderrClosed := make(chan struct{})
errs := bufio.NewScanner(stderrR)
go func() {
outs := bufio.NewScanner(stdoutR)
for outs.Scan() {
text := outs.Text()
sess.logger.Debug(title, "Dir", cmd.Dir, "Path", cmd.Path, "Args", cmd.Args, "Stdout", text)
stdout.WriteString(text + "\n")
}
}()
go func() {
errs := bufio.NewScanner(stderrR)
for errs.Scan() {
text := errs.Text()
sess.logger.Debug(title, "Dir", cmd.Dir, "Path", cmd.Path, "Args", cmd.Args, "Text", text)
stderr.WriteString(text + "\n")
}
close(stderrClosed)
}()

outs := bufio.NewScanner(stdoutR)
for outs.Scan() {
text := outs.Text()
sess.logger.Debug(title, "Dir", cmd.Dir, "Path", cmd.Path, "Args", cmd.Args, "Stdout", text)
stdout.WriteString(text + "\n")
}
<-stderrClosed

// Now wait for the command to finish. No matter what, return everything written to stdout and
// stderr, in addition to the resulting error, if any.
err = cmd.Wait()
Expand Down

0 comments on commit 8f69a7d

Please sign in to comment.