From dc62a0f4177f0516462fff58aab2787bbe766696 Mon Sep 17 00:00:00 2001
From: Lorna Song <lorna@hashicorp.com>
Date: Thu, 5 May 2022 12:20:17 -0400
Subject: [PATCH] Test for runTerraformCmd leaked go-routine

Currently, when runTerraformCmd is called, it starts a go-routine to kill the
Terraform CLI on context.Done(). However, when the Terraform CLI completes and
runTerraformCmd() finishes, the go-routine continues running unnecessarily.

If the caller cancels the context down the line, this will stop the go-routine
and it will log the error: "error from kill: os: process already finished" because
the Terraform CLI has already finished.

Added a test for this in cmd_default.go and cmd_linux.go. Have not tried it in
linux yet though.

When running with the race detector:
```
==================
WARNING: DATA RACE
Read at 0x00c0002516c8 by goroutine 7:
  bytes.(*Buffer).String()
      /usr/local/go/src/bytes/buffer.go:65 +0x36a
  github.com/hashicorp/terraform-exec/tfexec.Test_runTerraformCmd_default()
      /Users/lornasong/go/src/github.com/hashicorp/terraform-exec/tfexec/cmd_default_test.go:35 +0x360
  testing.tRunner()
  // truncated ...

Previous write at 0x00c0002516c8 by goroutine 8:
  bytes.(*Buffer).grow()
      /usr/local/go/src/bytes/buffer.go:147 +0x3b1
  bytes.(*Buffer).Write()
      /usr/local/go/src/bytes/buffer.go:172 +0xcd
  log.(*Logger).Output()
      /usr/local/go/src/log/log.go:184 +0x466
  log.(*Logger).Printf()
      /usr/local/go/src/log/log.go:191 +0x6e
  github.com/hashicorp/terraform-exec/tfexec.(*Terraform).runTerraformCmd.func1()
      /Users/lornasong/go/src/github.com/hashicorp/terraform-exec/tfexec/cmd_default.go:24 +0x2a5
  // truncated ...
==================
---
 tfexec/cmd_default_test.go | 39 ++++++++++++++++++++++++++++++++++++++
 tfexec/cmd_linux_test.go   | 36 +++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 tfexec/cmd_default_test.go
 create mode 100644 tfexec/cmd_linux_test.go

diff --git a/tfexec/cmd_default_test.go b/tfexec/cmd_default_test.go
new file mode 100644
index 00000000..0245ddaa
--- /dev/null
+++ b/tfexec/cmd_default_test.go
@@ -0,0 +1,39 @@
+//go:build !linux
+// +build !linux
+
+package tfexec
+
+import (
+	"bytes"
+	"context"
+	"log"
+	"strings"
+	"testing"
+	"time"
+)
+
+func Test_runTerraformCmd_default(t *testing.T) {
+	// Checks runTerraformCmd for race condition when using
+	// go test -race -run Test_runTerraformCmd_default ./tfexec
+	var buf bytes.Buffer
+
+	tf := &Terraform{
+		logger:   log.New(&buf, "", 0),
+		execPath: "echo",
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+
+	cmd := tf.buildTerraformCmd(ctx, nil, "hello tf-exec!")
+	err := tf.runTerraformCmd(ctx, cmd)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Cancel stops the leaked go routine which logs an error
+	cancel()
+	time.Sleep(time.Second)
+	if strings.Contains(buf.String(), "error from kill") {
+		t.Fatal("canceling context should not lead to logging an error")
+	}
+}
diff --git a/tfexec/cmd_linux_test.go b/tfexec/cmd_linux_test.go
new file mode 100644
index 00000000..471ddf57
--- /dev/null
+++ b/tfexec/cmd_linux_test.go
@@ -0,0 +1,36 @@
+package tfexec
+
+import (
+	"bytes"
+	"context"
+	"log"
+	"strings"
+	"testing"
+	"time"
+)
+
+func Test_runTerraformCmd_linux(t *testing.T) {
+	// Checks runTerraformCmd for race condition when using
+	// go test -race -run Test_runTerraformCmd_linux ./tfexec -tags=linux
+	var buf bytes.Buffer
+
+	tf := &Terraform{
+		logger:   log.New(&buf, "", 0),
+		execPath: "echo",
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+
+	cmd := tf.buildTerraformCmd(ctx, nil, "hello tf-exec!")
+	err := tf.runTerraformCmd(ctx, cmd)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Cancel stops the leaked go routine which logs an error
+	cancel()
+	time.Sleep(time.Second)
+	if strings.Contains(buf.String(), "error from kill") {
+		t.Fatal("canceling context should not lead to logging an error")
+	}
+}