Merge pull request #29825 from hashicorp/jbardin/no-panicwrap

Remove the use of panicwrap in the Terraform CLI

go.mod

@@ -64,7 +64,6 @@ require (
 	github.com/mitchellh/go-wordwrap v1.0.1
 	github.com/mitchellh/gox v1.0.1
 	github.com/mitchellh/mapstructure v1.1.2
-	github.com/mitchellh/panicwrap v1.0.0
 	github.com/mitchellh/reflectwalk v1.0.2
 	github.com/nishanths/exhaustive v0.2.3
 	github.com/packer-community/winrmcp v0.0.0-20180921211025-c76d91c1e7db

go.sum

@@ -521,8 +521,6 @@ github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0Qu
 github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
 github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQzvN1EDeE=
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
-github.com/mitchellh/panicwrap v1.0.0 h1:67zIyVakCIvcs69A0FGfZjBdPleaonSgGlXRSRlb6fE=
-github.com/mitchellh/panicwrap v1.0.0/go.mod h1:pKvZHwWrZowLUzftuFq7coarnxbBXU4aQh3N0BJOeeA=
 github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ=
 github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=

internal/backend/local/backend.go

@@ -14,6 +14,7 @@ import (
 	"github.com/hashicorp/terraform/internal/backend"
 	"github.com/hashicorp/terraform/internal/command/views"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/states/statemgr"
 	"github.com/hashicorp/terraform/internal/terraform"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -313,6 +314,7 @@ func (b *Local) Operation(ctx context.Context, op *backend.Operation) (*backend.
 
 	// Do it
 	go func() {
+		defer logging.PanicHandler()
 		defer done()
 		defer stop()
 		defer cancel()

internal/backend/local/backend_apply.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/backend"
 	"github.com/hashicorp/terraform/internal/command/views"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/plans"
 	"github.com/hashicorp/terraform/internal/states"
 	"github.com/hashicorp/terraform/internal/states/statefile"
@@ -156,6 +157,7 @@ func (b *Local) opApply(
 	var applyDiags tfdiags.Diagnostics
 	doneCh := make(chan struct{})
 	go func() {
+		defer logging.PanicHandler()
 		defer close(doneCh)
 		log.Printf("[INFO] backend/local: apply calling Apply")
 		applyState, applyDiags = lr.Core.Apply(plan, lr.Config)

internal/backend/local/backend_plan.go

@@ -6,6 +6,7 @@ import (
 	"log"
 
 	"github.com/hashicorp/terraform/internal/backend"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/plans"
 	"github.com/hashicorp/terraform/internal/plans/planfile"
 	"github.com/hashicorp/terraform/internal/states/statefile"
@@ -79,6 +80,7 @@ func (b *Local) opPlan(
 	var planDiags tfdiags.Diagnostics
 	doneCh := make(chan struct{})
 	go func() {
+		defer logging.PanicHandler()
 		defer close(doneCh)
 		log.Printf("[INFO] backend/local: plan calling Plan")
 		plan, planDiags = lr.Core.Plan(lr.Config, lr.InputState, lr.PlanOpts)

internal/backend/local/backend_refresh.go

@@ -7,6 +7,7 @@ import (
 	"os"
 
 	"github.com/hashicorp/terraform/internal/backend"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/states"
 	"github.com/hashicorp/terraform/internal/states/statemgr"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -77,6 +78,7 @@ func (b *Local) opRefresh(
 	var refreshDiags tfdiags.Diagnostics
 	doneCh := make(chan struct{})
 	go func() {
+		defer logging.PanicHandler()
 		defer close(doneCh)
 		newState, refreshDiags = lr.Core.Refresh(lr.Config, lr.InputState, lr.PlanOpts)
 		log.Printf("[INFO] backend/local: refresh calling Refresh")

internal/backend/remote/backend.go

@@ -18,6 +18,7 @@ import (
 	"github.com/hashicorp/terraform-svchost/disco"
 	"github.com/hashicorp/terraform/internal/backend"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/states/remote"
 	"github.com/hashicorp/terraform/internal/states/statemgr"
 	"github.com/hashicorp/terraform/internal/terraform"
@@ -755,6 +756,7 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
 
 	// Do it.
 	go func() {
+		defer logging.PanicHandler()
 		defer done()
 		defer stop()
 		defer cancel()

internal/backend/remote/backend_common.go

@@ -13,6 +13,7 @@ import (
 
 	tfe "github.com/hashicorp/go-tfe"
 	"github.com/hashicorp/terraform/internal/backend"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/plans"
 	"github.com/hashicorp/terraform/internal/terraform"
 )
@@ -464,6 +465,8 @@ func (b *Remote) confirm(stopCtx context.Context, op *backend.Operation, opts *t
 	result := make(chan error, 2)
 
 	go func() {
+		defer logging.PanicHandler()
+
 		// Make sure we cancel doneCtx before we return
 		// so the input command is also canceled.
 		defer cancel()

internal/backend/remote/backend_plan.go

@@ -17,6 +17,7 @@ import (
 	tfe "github.com/hashicorp/go-tfe"
 	version "github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform/internal/backend"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/plans"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
@@ -319,6 +320,8 @@ in order to capture the filesystem context the remote workspace expects:
 	// cancellable after that period, we attempt to cancel it.
 	if lockTimeout := op.StateLocker.Timeout(); lockTimeout > 0 {
 		go func() {
+			defer logging.PanicHandler()
+
 			select {
 			case <-stopCtx.Done():
 				return

internal/command/console.go

@@ -3,11 +3,11 @@ package command
 import (
 	"bufio"
 	"fmt"
+	"os"
 	"strings"
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/backend"
-	"github.com/hashicorp/terraform/internal/helper/wrappedstreams"
 	"github.com/hashicorp/terraform/internal/repl"
 	"github.com/hashicorp/terraform/internal/terraform"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -113,8 +113,8 @@ func (c *ConsoleCommand) Run(args []string) int {
 
 	// Set up the UI so we can output directly to stdout
 	ui := &cli.BasicUi{
-		Writer:      wrappedstreams.Stdout(),
+		Writer:      os.Stdout,
-		ErrorWriter: wrappedstreams.Stderr(),
+		ErrorWriter: os.Stderr,
 	}
 
 	evalOpts := &terraform.EvalOpts{}
@@ -164,7 +164,7 @@ func (c *ConsoleCommand) Run(args []string) int {
 
 func (c *ConsoleCommand) modePiped(session *repl.Session, ui cli.Ui) int {
 	var lastResult string
-	scanner := bufio.NewScanner(wrappedstreams.Stdin())
+	scanner := bufio.NewScanner(os.Stdin)
 	for scanner.Scan() {
 		result, exit, diags := session.Handle(strings.TrimSpace(scanner.Text()))
 		if diags.HasErrors() {

internal/command/console_interactive.go

@@ -9,8 +9,8 @@ package command
 import (
 	"fmt"
 	"io"
+	"os"
 
-	"github.com/hashicorp/terraform/internal/helper/wrappedreadline"
 	"github.com/hashicorp/terraform/internal/repl"
 
 	"github.com/chzyer/readline"
@@ -19,12 +19,15 @@ import (
 
 func (c *ConsoleCommand) modeInteractive(session *repl.Session, ui cli.Ui) int {
 	// Configure input
-	l, err := readline.NewEx(wrappedreadline.Override(&readline.Config{
+	l, err := readline.NewEx(&readline.Config{
 		Prompt:            "> ",
 		InterruptPrompt:   "^C",
 		EOFPrompt:         "exit",
 		HistorySearchFold: true,
-	}))
+		Stdin:             os.Stdin,
+		Stdout:            os.Stdout,
+		Stderr:            os.Stderr,
+	})
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf(
 			"Error initializing console: %s",

internal/command/login.go

@@ -22,6 +22,7 @@ import (
 	"github.com/hashicorp/terraform-svchost/disco"
 	"github.com/hashicorp/terraform/internal/command/cliconfig"
 	"github.com/hashicorp/terraform/internal/httpclient"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/terraform"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 
@@ -450,6 +451,7 @@ func (c *LoginCommand) interactiveGetTokenByCode(hostname svchost.Hostname, cred
 		}),
 	}
 	go func() {
+		defer logging.PanicHandler()
 		err := server.Serve(listener)
 		if err != nil && err != http.ErrServerClosed {
 			diags = diags.Append(tfdiags.Sourceless(

internal/helper/wrappedreadline/wrappedreadline.go

@@ -1,77 +0,0 @@
-// wrappedreadline is a package that has helpers for interacting with
-// readline from a panicwrap executable.
-//
-// panicwrap overrides the standard file descriptors so that the child process
-// no longer looks like a TTY. The helpers here access the extra file descriptors
-// passed by panicwrap to fix that.
-//
-// panicwrap should be checked for with panicwrap.Wrapped before using this
-// librar, since this library won't adapt if the binary is not wrapped.
-package wrappedreadline
-
-import (
-	"runtime"
-
-	"github.com/chzyer/readline"
-
-	"github.com/hashicorp/terraform/internal/helper/wrappedstreams"
-)
-
-// Override overrides the values in readline.Config that need to be
-// set with wrapped values.
-func Override(cfg *readline.Config) *readline.Config {
-	cfg.Stdin = wrappedstreams.Stdin()
-	cfg.Stdout = wrappedstreams.Stdout()
-	cfg.Stderr = wrappedstreams.Stderr()
-
-	cfg.FuncGetWidth = TerminalWidth
-	cfg.FuncIsTerminal = IsTerminal
-
-	rm := RawMode{StdinFd: int(wrappedstreams.Stdin().Fd())}
-	cfg.FuncMakeRaw = rm.Enter
-	cfg.FuncExitRaw = rm.Exit
-
-	return cfg
-}
-
-// IsTerminal determines if this process is attached to a TTY.
-func IsTerminal() bool {
-	// Windows is always a terminal
-	if runtime.GOOS == "windows" {
-		return true
-	}
-
-	// Same implementation as readline but with our custom fds
-	return readline.IsTerminal(int(wrappedstreams.Stdin().Fd())) &&
-		(readline.IsTerminal(int(wrappedstreams.Stdout().Fd())) ||
-			readline.IsTerminal(int(wrappedstreams.Stderr().Fd())))
-}
-
-// TerminalWidth gets the terminal width in characters.
-func TerminalWidth() int {
-	if runtime.GOOS == "windows" {
-		return readline.GetScreenWidth()
-	}
-
-	return getWidth()
-}
-
-// RawMode is a helper for entering and exiting raw mode.
-type RawMode struct {
-	StdinFd int
-
-	state *readline.State
-}
-
-func (r *RawMode) Enter() (err error) {
-	r.state, err = readline.MakeRaw(r.StdinFd)
-	return err
-}
-
-func (r *RawMode) Exit() error {
-	if r.state == nil {
-		return nil
-	}
-
-	return readline.Restore(r.StdinFd, r.state)
-}

internal/helper/wrappedreadline/wrappedreadline_unix.go

@@ -1,47 +0,0 @@
-//go:build darwin || dragonfly || freebsd || (linux && !appengine) || netbsd || openbsd
-// +build darwin dragonfly freebsd linux,!appengine netbsd openbsd
-
-package wrappedreadline
-
-import (
-	"syscall"
-	"unsafe"
-
-	"github.com/hashicorp/terraform/internal/helper/wrappedstreams"
-)
-
-// getWidth impl for Unix
-func getWidth() int {
-	stdoutFd := int(wrappedstreams.Stdout().Fd())
-	stderrFd := int(wrappedstreams.Stderr().Fd())
-
-	w := getWidthFd(stdoutFd)
-	if w < 0 {
-		w = getWidthFd(stderrFd)
-	}
-
-	return w
-}
-
-type winsize struct {
-	Row    uint16
-	Col    uint16
-	Xpixel uint16
-	Ypixel uint16
-}
-
-// get width of the terminal
-func getWidthFd(stdoutFd int) int {
-	ws := &winsize{}
-	retCode, _, errno := syscall.Syscall(syscall.SYS_IOCTL,
-		uintptr(stdoutFd),
-		uintptr(syscall.TIOCGWINSZ),
-		uintptr(unsafe.Pointer(ws)))
-
-	if int(retCode) == -1 {
-		_ = errno
-		return -1
-	}
-
-	return int(ws.Col)
-}

internal/helper/wrappedreadline/wrappedreadline_windows.go

@@ -1,9 +0,0 @@
-//go:build windows
-// +build windows
-
-package wrappedreadline
-
-// getWidth impl for other
-func getWidth() int {
-	return 0
-}

internal/helper/wrappedstreams/streams.go

@@ -1,44 +0,0 @@
-// Package wrappedstreams provides access to the standard OS streams
-// (stdin, stdout, stderr) even if wrapped under panicwrap.
-package wrappedstreams
-
-import (
-	"os"
-
-	"github.com/mitchellh/panicwrap"
-)
-
-// Stdin returns the true stdin of the process.
-func Stdin() *os.File {
-	stdin, _, _ := fds()
-	return stdin
-}
-
-// Stdout returns the true stdout of the process.
-func Stdout() *os.File {
-	_, stdout, _ := fds()
-	return stdout
-}
-
-// Stderr returns the true stderr of the process.
-func Stderr() *os.File {
-	_, _, stderr := fds()
-	return stderr
-}
-
-func fds() (stdin, stdout, stderr *os.File) {
-	stdin, stdout, stderr = os.Stdin, os.Stdout, os.Stderr
-	if panicwrap.Wrapped(nil) {
-		initPlatform()
-		stdin, stdout, stderr = wrappedStdin, wrappedStdout, wrappedStderr
-	}
-	return
-}
-
-// These are the wrapped standard streams. These are set up by the
-// platform specific code in initPlatform.
-var (
-	wrappedStdin  *os.File
-	wrappedStdout *os.File
-	wrappedStderr *os.File
-)

internal/helper/wrappedstreams/streams_other.go

@@ -1,22 +0,0 @@
-//go:build !windows
-// +build !windows
-
-package wrappedstreams
-
-import (
-	"os"
-	"sync"
-)
-
-var initOnce sync.Once
-
-func initPlatform() {
-	// These must be initialized lazily, once it's been determined that this is
-	// a wrapped process.
-	initOnce.Do(func() {
-		// The standard streams are passed in via extra file descriptors.
-		wrappedStdin = os.NewFile(uintptr(3), "stdin")
-		wrappedStdout = os.NewFile(uintptr(4), "stdout")
-		wrappedStderr = os.NewFile(uintptr(5), "stderr")
-	})
-}

internal/helper/wrappedstreams/streams_windows.go

@@ -1,53 +0,0 @@
-//go:build windows
-// +build windows
-
-package wrappedstreams
-
-import (
-	"log"
-	"os"
-	"syscall"
-)
-
-func initPlatform() {
-	wrappedStdin = openConsole("CONIN$", os.Stdin)
-	wrappedStdout = openConsole("CONOUT$", os.Stdout)
-	wrappedStderr = wrappedStdout
-}
-
-// openConsole opens a console handle, using a backup if it fails.
-// This is used to get the exact console handle instead of the redirected
-// handles from panicwrap.
-func openConsole(name string, backup *os.File) *os.File {
-	// Convert to UTF16
-	path, err := syscall.UTF16PtrFromString(name)
-	if err != nil {
-		log.Printf("[ERROR] wrappedstreams: %s", err)
-		return backup
-	}
-
-	// Determine the share mode
-	var shareMode uint32
-	switch name {
-	case "CONIN$":
-		shareMode = syscall.FILE_SHARE_READ
-	case "CONOUT$":
-		shareMode = syscall.FILE_SHARE_WRITE
-	}
-
-	// Get the file
-	h, err := syscall.CreateFile(
-		path,
-		syscall.GENERIC_READ|syscall.GENERIC_WRITE,
-		shareMode,
-		nil,
-		syscall.OPEN_EXISTING,
-		0, 0)
-	if err != nil {
-		log.Printf("[ERROR] wrappedstreams: %s", err)
-		return backup
-	}
-
-	// Create the Go file
-	return os.NewFile(uintptr(h), name)
-}

internal/logging/panic.go

@@ -2,75 +2,51 @@ package logging
 
 import (
 	"fmt"
-	"io"
-	"io/ioutil"
 	"os"
+	"runtime/debug"
 	"strings"
 	"sync"
 
 	"github.com/hashicorp/go-hclog"
-	"github.com/mitchellh/panicwrap"
 )
 
 // This output is shown if a panic happens.
 const panicOutput = `
-
 !!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
 Terraform crashed! This is always indicative of a bug within Terraform.
-A crash log has been placed at %[1]q relative to your current
+Please report the crash with Terraform[1] so that we can fix this.
-working directory. It would be immensely helpful if you could please
-report the crash with Terraform[1] so that we can fix this.
 
-When reporting bugs, please include your terraform version. That
+When reporting bugs, please include your terraform version, the stack trace
-information is available on the first line of crash.log. You can also
+shown below, and any additional information which may help replicate the issue.
-get it by running 'terraform --version' on the command line.
-
-SECURITY WARNING: the %[1]q file that was created may contain 
-sensitive information that must be redacted before it is safe to share 
-on the issue tracker.
 
 [1]: https://github.com/hashicorp/terraform/issues
 
 !!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!
+
 `
 
-// panicHandler is what is called by panicwrap when a panic is encountered
+// PanicHandler is called to recover from an internal panic in Terraform, and
-// within Terraform. It is guaranteed to run after the resulting process has
+// augments the standard stack trace with a more user friendly error message.
-// exited so we can take the log file, add in the panic, and store it
+// PanicHandler must be called as a defered function, and must be the first
-// somewhere locally.
+// defer called at the start of a new goroutine.
-func PanicHandler(tmpLogPath string) panicwrap.HandlerFunc {
+func PanicHandler() {
-	return func(m string) {
+	recovered := recover()
-		// Create the crash log file where we'll write the logs
+	if recovered == nil {
-		f, err := ioutil.TempFile(".", "crash.*.log")
+		return
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "Failed to create crash log file: %s", err)
-			return
-		}
-		defer f.Close()
-
-		tmpLog, err := os.Open(tmpLogPath)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "Failed to open log file %q: %v\n", tmpLogPath, err)
-			return
-		}
-		defer tmpLog.Close()
-
-		// Copy the contents to the crash file. This will include
-		// the panic that just happened.
-		if _, err = io.Copy(f, tmpLog); err != nil {
-			fmt.Fprintf(os.Stderr, "Failed to write crash log: %s", err)
-			return
-		}
-
-		// add the trace back to the log
-		f.WriteString("\n" + m)
-
-		// Tell the user a crash occurred in some helpful way that
-		// they'll hopefully notice.
-		fmt.Printf("\n\n")
-		fmt.Printf(panicOutput, f.Name())
 	}
+
+	fmt.Fprint(os.Stderr, panicOutput)
+	fmt.Fprint(os.Stderr, recovered)
+
+	// When called from a deferred function, debug.PrintStack will include the
+	// full stack from the point of the pending panic.
+	debug.PrintStack()
+
+	// An exit code of 11 keeps us out of the way of the detailed exitcodes
+	// from plan, and also happens to be the same code as SIGSEGV which is
+	// roughly the same type of condition that causes most panics.
+	os.Exit(11)
 }
 
 const pluginPanicOutput = `
@@ -181,13 +157,5 @@ func (l *logPanicWrapper) Debug(msg string, args ...interface{}) {
 		l.panicRecorder(msg)
 	}
 
-	// If we have logging turned on, we need to prevent panicwrap from seeing
-	// this as a core panic. This can be done by obfuscating the panic error
-	// line.
-	if panicPrefix {
-		colon := strings.Index(msg, ":")
-		msg = strings.ToUpper(msg[:colon]) + msg[colon:]
-	}
-
 	l.Logger.Debug(msg, args...)
 }

internal/logging/panic_test.go

@@ -1,12 +1,9 @@
 package logging
 
 import (
-	"bytes"
 	"fmt"
 	"strings"
 	"testing"
-
-	"github.com/hashicorp/go-hclog"
 )
 
 func TestPanicRecorder(t *testing.T) {
@@ -52,31 +49,3 @@ func TestPanicLimit(t *testing.T) {
 		}
 	}
 }
-
-func TestLogPanicWrapper(t *testing.T) {
-	var buf bytes.Buffer
-	logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{
-		Name:        "test",
-		Level:       hclog.Debug,
-		Output:      &buf,
-		DisableTime: true,
-	})
-
-	wrapped := (&logPanicWrapper{
-		Logger: logger,
-	}).Named("test")
-
-	wrapped.Debug("panic: invalid foo of bar")
-	wrapped.Debug("\tstack trace")
-
-	expected := `[DEBUG] test.test: PANIC: invalid foo of bar
-[DEBUG] test.test: 	stack trace
-`
-
-	got := buf.String()
-
-	if expected != got {
-		t.Fatalf("Expected:\n%q\nGot:\n%q", expected, got)
-	}
-
-}

internal/terminal/panicwrap_ugh.go

@@ -1,78 +0,0 @@
-package terminal
-
-import "os"
-
-// This file has some annoying nonsense to, yet again, work around the
-// panicwrap hack.
-//
-// Specifically, typically when we're running Terraform the stderr handle is
-// not directly connected to the terminal but is instead a pipe into a parent
-// process gathering up the output just in case a panic message appears.
-// However, this package needs to know whether the _real_ stderr is connected
-// to a terminal and what its width is.
-//
-// To work around that, we'll first initialize the terminal in the parent
-// process, and then capture information about stderr into an environment
-// variable so we can pass it down to the child process. The child process
-// will then use the environment variable to pretend that the panicwrap pipe
-// has the same characteristics as the terminal that it's indirectly writing
-// to.
-//
-// This file has some helpers for implementing that awkward handshake, but the
-// handshake itself is in package main, interspersed with all of the other
-// panicwrap machinery.
-//
-// You might think that the code in helper/wrappedstreams could avoid this
-// problem, but that package is broken on Windows: it always fails to recover
-// the real stderr, and it also gets an incorrect result if the user was
-// redirecting or piping stdout/stdin. So... we have this hack instead, which
-// gets a correct result even on Windows and even with I/O redirection.
-
-// StateForAfterPanicWrap is part of the workaround for panicwrap that
-// captures some characteristics of stderr that the caller can pass to the
-// panicwrap child process somehow and then use ReinitInsidePanicWrap.
-func (s *Streams) StateForAfterPanicWrap() *PrePanicwrapState {
-	return &PrePanicwrapState{
-		StderrIsTerminal: s.Stderr.IsTerminal(),
-		StderrWidth:      s.Stderr.Columns(),
-	}
-}
-
-// ReinitInsidePanicwrap is part of the workaround for panicwrap that
-// produces a Streams containing a potentially-lying Stderr that might
-// claim to be a terminal even if it's actually a pipe connected to the
-// parent process.
-//
-// That's an okay lie in practice because the parent process will copy any
-// data it recieves via that pipe verbatim to the real stderr anyway. (The
-// original call to Init in the parent process should've already done any
-// necessary modesetting on the Stderr terminal, if any.)
-//
-// The state argument can be nil if we're not running in panicwrap mode,
-// in which case this function behaves exactly the same as Init.
-func ReinitInsidePanicwrap(state *PrePanicwrapState) (*Streams, error) {
-	ret, err := Init()
-	if err != nil {
-		return ret, err
-	}
-	if state != nil {
-		// A lying stderr, then.
-		ret.Stderr = &OutputStream{
-			File: ret.Stderr.File,
-			isTerminal: func(f *os.File) bool {
-				return state.StderrIsTerminal
-			},
-			getColumns: func(f *os.File) int {
-				return state.StderrWidth
-			},
-		}
-	}
-	return ret, nil
-}
-
-// PrePanicwrapState is a horrible thing we use to work around panicwrap,
-// related to both Streams.StateForAfterPanicWrap and ReinitInsidePanicwrap.
-type PrePanicwrapState struct {
-	StderrIsTerminal bool
-	StderrWidth      int
-}

internal/terraform/context.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs"
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/providers"
 	"github.com/hashicorp/terraform/internal/provisioners"
 	"github.com/hashicorp/terraform/internal/states"
@@ -264,6 +265,8 @@ func (c *Context) watchStop(walker *ContextGraphWalker) (chan struct{}, <-chan s
 	done := c.runContext.Done()
 
 	go func() {
+		defer logging.PanicHandler()
+
 		defer close(wait)
 		// Wait for a stop or completion
 		select {

internal/terraform/graph.go

@@ -4,6 +4,7 @@ import (
 	"log"
 	"strings"
 
+	"github.com/hashicorp/terraform/internal/logging"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 
 	"github.com/hashicorp/terraform/internal/addrs"
@@ -39,6 +40,10 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics {
 
 	// Walk the graph.
 	walkFn := func(v dag.Vertex) (diags tfdiags.Diagnostics) {
+		// the walkFn is called asynchronously, and needs to be recovered
+		// separately in the case of a panic.
+		defer logging.PanicHandler()
+
 		log.Printf("[TRACE] vertex %q: starting visit (%T)", dag.VertexName(v), v)
 
 		defer func() {

main.go

@@ -3,7 +3,6 @@ package main
 import (
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"log"
 	"net"
 	"os"
@@ -24,7 +23,6 @@ import (
 	"github.com/mattn/go-shellwords"
 	"github.com/mitchellh/cli"
 	"github.com/mitchellh/colorstring"
-	"github.com/mitchellh/panicwrap"
 
 	backendInit "github.com/hashicorp/terraform/internal/backend/init"
 )
@@ -35,12 +33,6 @@ const (
 
 	// The parent process will create a file to collect crash logs
 	envTmpLogPath = "TF_TEMP_LOG_PATH"
-
-	// Environment variable name used for smuggling true stderr terminal
-	// settings into a panicwrap child process. This is an implementation
-	// detail, subject to change in future, and should not ever be directly
-	// set by an end-user.
-	envTerminalPanicwrapWorkaround = "TF_PANICWRAP_STDERR"
 )
 
 // ui wraps the primary output cli.Ui, and redirects Warn calls to Output
@@ -54,67 +46,6 @@ func (u *ui) Warn(msg string) {
 	u.Ui.Output(msg)
 }
 
-func main() {
-	os.Exit(realMain())
-}
-
-func realMain() int {
-	var wrapConfig panicwrap.WrapConfig
-
-	// don't re-exec terraform as a child process for easier debugging
-	if os.Getenv("TF_FORK") == "0" {
-		return wrappedMain()
-	}
-
-	if !panicwrap.Wrapped(&wrapConfig) {
-		// We always send logs to a temporary file that we use in case
-		// there is a panic. Otherwise, we delete it.
-		logTempFile, err := ioutil.TempFile("", "terraform-log")
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "Couldn't set up logging tempfile: %s", err)
-			return 1
-		}
-		// Now that we have the file, close it and leave it for the wrapped
-		// process to write to.
-		logTempFile.Close()
-		defer os.Remove(logTempFile.Name())
-
-		// store the path in the environment for the wrapped executable
-		os.Setenv(envTmpLogPath, logTempFile.Name())
-
-		// We also need to do our terminal initialization before we fork,
-		// because the child process doesn't necessarily have access to
-		// the true stderr in order to initialize it.
-		streams, err := terminal.Init()
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "Failed to initialize terminal: %s", err)
-			return 1
-		}
-
-		// We need the child process to behave _as if_ connected to the real
-		// stderr, even though panicwrap is about to add a pipe in the way,
-		// so we'll smuggle the true stderr information in an environment
-		// varible.
-		streamState := streams.StateForAfterPanicWrap()
-		os.Setenv(envTerminalPanicwrapWorkaround, fmt.Sprintf("%t:%d", streamState.StderrIsTerminal, streamState.StderrWidth))
-
-		// Create the configuration for panicwrap and wrap our executable
-		wrapConfig.Handler = logging.PanicHandler(logTempFile.Name())
-		wrapConfig.IgnoreSignals = ignoreSignals
-		wrapConfig.ForwardSignals = forwardSignals
-		exitStatus, err := panicwrap.Wrap(&wrapConfig)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "Couldn't start Terraform: %s", err)
-			return 1
-		}
-
-		return exitStatus
-	}
-
-	// Call the real main
-	return wrappedMain()
-}
-
 func init() {
 	Ui = &ui{&cli.BasicUi{
 		Writer:      os.Stdout,
@@ -123,7 +54,13 @@ func init() {
 	}}
 }
 
-func wrappedMain() int {
+func main() {
+	os.Exit(realMain())
+}
+
+func realMain() int {
+	defer logging.PanicHandler()
+
 	var err error
 
 	tmpLogPath := os.Getenv(envTmpLogPath)
@@ -145,19 +82,7 @@ func wrappedMain() int {
 	log.Printf("[INFO] Go runtime version: %s", runtime.Version())
 	log.Printf("[INFO] CLI args: %#v", os.Args)
 
-	// This is the recieving end of our workaround to retain the metadata
+	streams, err := terminal.Init()
-	// about the real stderr even though we're talking to it via the panicwrap
-	// pipe. See the call to StateForAfterPanicWrap above for the producer
-	// part of this.
-	var streamState *terminal.PrePanicwrapState
-	if raw := os.Getenv(envTerminalPanicwrapWorkaround); raw != "" {
-		streamState = &terminal.PrePanicwrapState{}
-		if _, err := fmt.Sscanf(raw, "%t:%d", &streamState.StderrIsTerminal, &streamState.StderrWidth); err != nil {
-			log.Printf("[WARN] %s is set but is incorrectly-formatted: %s", envTerminalPanicwrapWorkaround, err)
-			streamState = nil // leave it unset for a normal init, then
-		}
-	}
-	streams, err := terminal.ReinitInsidePanicwrap(streamState)
 	if err != nil {
 		Ui.Error(fmt.Sprintf("Failed to configure the terminal: %s", err))
 		return 1
@@ -391,9 +316,7 @@ func wrappedMain() int {
 	// plugins crashing
 	if exitCode != 0 {
 		for _, panicLog := range logging.PluginPanics() {
-			// we don't write this to Error, or else panicwrap will think this
+			Ui.Error(panicLog)
-			// process panicked
-			Ui.Info(panicLog)
 		}
 	}
 

main_test.go

@@ -130,7 +130,7 @@ func TestMain_cliArgsFromEnv(t *testing.T) {
 			// Run it!
 			os.Args = args
 			testCommand.Args = nil
-			exit := wrappedMain()
+			exit := realMain()
 			if (exit != 0) != tc.Err {
 				t.Fatalf("bad: %d", exit)
 			}
@@ -237,7 +237,7 @@ func TestMain_cliArgsFromEnvAdvanced(t *testing.T) {
 			// Run it!
 			os.Args = args
 			testCommand.Args = nil
-			exit := wrappedMain()
+			exit := realMain()
 			if (exit != 0) != tc.Err {
 				t.Fatalf("unexpected exit status %d; want 0", exit)
 			}
@@ -275,7 +275,7 @@ func TestMain_autoComplete(t *testing.T) {
 
 	// Run it!
 	os.Args = []string{"terraform", "terraform", "versio"}
-	exit := wrappedMain()
+	exit := realMain()
 	if exit != 0 {
 		t.Fatalf("unexpected exit status %d; want 0", exit)
 	}

scripts/debug-terraform

@@ -18,11 +18,6 @@
 
 set -eu
 
-# Make sure we're debugging the process where the code is actually running.
-# (This also, as a side effect, causes raw logs to go directly to stderr,
-# and panics to be expressed directly, since we lose the log/panic wrapper.)
-export TF_FORK=0
-
 echo "Launching Terraform in a headless debug session"
 echo "Connect to it using: dlv connect 127.0.0.1:2345"
 echo "(Terraform takes a long time to build and launch in this mode; some logs will appear below)"