Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test for runTerraformCmd leaked go-routine #299

Merged
merged 8 commits into from May 20, 2022
Merged

Test for runTerraformCmd leaked go-routine #299

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
400f9d7
Update TestContext_sleepTimeoutExpired to check for canceling within …
lornasong May 9, 2022
b4e272e
Remove runTerraformCmd check for cmd.ProcessState
lornasong May 9, 2022
c3ccdc3
Read logs from Stderr/out Pipe to avoid hanging
lornasong May 10, 2022
dc62a0f
Test for runTerraformCmd leaked go-routine
lornasong May 5, 2022
53bfc40
Use CommandContext to kill instead of manually doing it
kylecarbs Feb 2, 2022
11f14d2
Fix EOF error check to use error.Is()
lornasong May 17, 2022
594b411
Update tests to use separate string.Builder-s for stdout and stderr
lornasong May 20, 2022
5a93b1e
Fixes to runTerraformCmd for race conditions
lornasong May 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion tfexec/cmd.go
View file
Open in desktop
Expand Up @@ -233,7 +233,21 @@ func mergeWriters(writers ...io.Writer) io.Writer {
return io.MultiWriter(compact...)
}

func writeOutput(r io.ReadCloser, w io.Writer) error {
func writeOutput(ctx context.Context, r io.ReadCloser, w io.Writer) error {
// ReadBytes will block until bytes are read, which can cause a delay in
// returning even if the command's context has been canceled. Use a separate
// goroutine to prompt ReadBytes to return on cancel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the comment ❤️

closeCtx, closeCancel := context.WithCancel(ctx)
defer closeCancel()
go func() {
select {
case <-ctx.Done():
r.Close()
case <-closeCtx.Done():
return
}
}()

buf := bufio.NewReader(r)
for {
line, err := buf.ReadBytes('\n')
Expand Down
37 changes: 22 additions & 15 deletions tfexec/cmd_default.go
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"os/exec"
"strings"
"sync"
)

func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error {
Expand Down Expand Up @@ -46,15 +47,24 @@ func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error {
return tf.wrapExitError(ctx, err, "")
}

exitChLen := 2
exitCh := make(chan error, exitChLen)
var errStdout, errStderr error
var wg sync.WaitGroup
wg.Add(1)
go func() {
exitCh <- writeOutput(stdoutPipe, stdoutWriter)
defer wg.Done()
errStdout = writeOutput(ctx, stdoutPipe, stdoutWriter)
}()

wg.Add(1)
go func() {
exitCh <- writeOutput(stderrPipe, stderrWriter)
defer wg.Done()
errStderr = writeOutput(ctx, stderrPipe, stderrWriter)
}()

// Reads from pipes must be completed before calling cmd.Wait(). Otherwise
// can cause a race condition
wg.Wait()

err = cmd.Wait()
if err == nil && ctx.Err() != nil {
err = ctx.Err()
Expand All @@ -63,16 +73,13 @@ func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error {
return tf.wrapExitError(ctx, err, errBuf.String())
}

// Wait for the logs to finish writing
counter := 0
for {
counter++
err := <-exitCh
if err != nil && err != context.Canceled {
return tf.wrapExitError(ctx, err, errBuf.String())
}
if counter >= exitChLen {
return ctx.Err()
}
// Return error if there was an issue reading the std out/err
if errStdout != nil && ctx.Err() != nil {
return tf.wrapExitError(ctx, errStdout, errBuf.String())
}
if errStderr != nil && ctx.Err() != nil {
return tf.wrapExitError(ctx, errStderr, errBuf.String())
}

return nil
}
37 changes: 22 additions & 15 deletions tfexec/cmd_linux.go
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"os/exec"
"strings"
"sync"
"syscall"
)

Expand Down Expand Up @@ -51,15 +52,24 @@ func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error {
return tf.wrapExitError(ctx, err, "")
}

exitChLen := 2
exitCh := make(chan error, exitChLen)
var errStdout, errStderr error
var wg sync.WaitGroup
wg.Add(1)
go func() {
exitCh <- writeOutput(stdoutPipe, stdoutWriter)
defer wg.Done()
errStdout = writeOutput(ctx, stdoutPipe, stdoutWriter)
}()

wg.Add(1)
go func() {
exitCh <- writeOutput(stderrPipe, stderrWriter)
defer wg.Done()
errStderr = writeOutput(ctx, stderrPipe, stderrWriter)
}()

// Reads from pipes must be completed before calling cmd.Wait(). Otherwise
// can cause a race condition
wg.Wait()

err = cmd.Wait()
if err == nil && ctx.Err() != nil {
err = ctx.Err()
Expand All @@ -68,16 +78,13 @@ func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error {
return tf.wrapExitError(ctx, err, errBuf.String())
}

// Wait for the logs to finish writing
counter := 0
for {
counter++
err := <-exitCh
if err != nil && err != context.Canceled {
return tf.wrapExitError(ctx, err, errBuf.String())
}
if counter >= exitChLen {
return ctx.Err()
}
// Return error if there was an issue reading the std out/err
if errStdout != nil && ctx.Err() != nil {
return tf.wrapExitError(ctx, errStdout, errBuf.String())
}
if errStderr != nil && ctx.Err() != nil {
return tf.wrapExitError(ctx, errStderr, errBuf.String())
}

return nil
}