Drain std{out,err} when a plugin fails to load.

If a plugin fails to load after we've set up the goroutines that copy
from its std{out,err} streams, then those goroutines can end up writing
to a closed event channel. This change ensures that we properly drain
those streams in this case.

pkg/engine/preview.go

@@ -47,7 +47,11 @@ func (eng *Engine) Preview(stack tokens.QName, events chan<- Event, opts Preview
 
 	defer close(events)
 
-	result, err := eng.plan(info, deployOpts)
+	return eng.previewLatest(info, deployOpts)
+}
+
+func (eng *Engine) previewLatest(info *planContext, opts deployOptions) error {
+	result, err := eng.plan(info, opts)
 	if err != nil {
 		return err
 	}
@@ -57,10 +61,11 @@ func (eng *Engine) Preview(stack tokens.QName, events chan<- Event, opts Preview
 			return err
 		}
 	}
-	if !diag.Success() {
+	if !opts.Diag.Success() {
 		// If any error occurred while walking the plan, be sure to let the developer know.  Otherwise,
 		// although error messages may have spewed to the output, the final lines of the plan may look fine.
 		return errors.New("One or more errors occurred during the creation of this preview")
 	}
 	return nil
+
 }

pkg/resource/plugin/plugin.go

@@ -61,6 +61,13 @@ func newPlugin(ctx *Context, bin string, prefix string, args []string) (*plugin,
 	}
 	contract.Assert(plug != nil)
 
+	// If we did not successfully launch the plugin, we still need to wait for stderr and stdout to drain.
+	defer func() {
+		if plug.Conn == nil {
+			contract.IgnoreError(plug.Close())
+		}
+	}()
+
 	// For now, we will spawn goroutines that will spew STDOUT/STDERR to the relevant diag streams.
 	runtrace := func(t io.Reader, stderr bool, done chan<- bool) {
 		reader := bufio.NewReader(t)
@@ -79,10 +86,9 @@ func newPlugin(ctx *Context, bin string, prefix string, args []string) (*plugin,
 		close(done)
 	}
 
-	stderrDone, stdoutDone := make(chan bool), make(chan bool)
-	plug.stderrDone, plug.stdoutDone = stderrDone, stdoutDone
-
 	// Set up a tracer on stderr before going any further, since important errors might get communicated this way.
+	stderrDone := make(chan bool)
+	plug.stderrDone = stderrDone
 	go runtrace(plug.Stderr, true, stderrDone)
 
 	// Now that we have a process, we expect it to write a single line to STDOUT: the port it's listening on.  We only
@@ -114,6 +120,8 @@ func newPlugin(ctx *Context, bin string, prefix string, args []string) (*plugin,
 	}
 
 	// After reading the port number, set up a tracer on stdout just so other output doesn't disappear.
+	stdoutDone := make(chan bool)
+	plug.stdoutDone = stdoutDone
 	go runtrace(plug.Stdout, false, stdoutDone)
 
 	// Now that we have the port, go ahead and create a gRPC client connection to it.
@@ -196,8 +204,10 @@ func execPlugin(bin string, args []string) (*plugin, error) {
 }
 
 func (p *plugin) Close() error {
-	closerr := p.Conn.Close()
-	contract.IgnoreError(closerr)
+	if p.Conn != nil {
+		closerr := p.Conn.Close()
+		contract.IgnoreError(closerr)
+	}
 
 	// IDEA: consider a more graceful termination than just SIGKILL.
 	err := p.Proc.Kill()