Merge pull request #7579 from hashicorp/fix_7525

fix race condition causing hang

go.mod

@@ -75,6 +75,7 @@ require (
 	github.com/mitchellh/go-fs v0.0.0-20180402234041-7b48fa161ea7
 	github.com/mitchellh/go-homedir v1.0.0
 	github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed
+	github.com/mitchellh/iochan v1.0.0
 	github.com/mitchellh/mapstructure v0.0.0-20180111000720-b4575eea38cc
 	github.com/mitchellh/panicwrap v0.0.0-20170106182340-fce601fe5557
 	github.com/mitchellh/prefixedio v0.0.0-20151214002211-6e6954073784

go.sum

@@ -278,6 +278,7 @@ github.com/mitchellh/go-testing-interface v1.0.0/go.mod h1:kRemZodwjscx+RGhAo8eI
 github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed h1:FI2NIv6fpef6BQl2u3IZX/Cj20tfypRF4yd+uaHOMtI=
 github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed/go.mod h1:3rdaFaCv4AyBgu5ALFM0+tSuHrBh6v692nyQe3ikrq0=
 github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS42BGNg=
+github.com/mitchellh/iochan v1.0.0 h1:C+X3KsSTLFVBr/tK1eYN/vs4rJcvsiLU338UhYPJWeY=
 github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY=
 github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
 github.com/mitchellh/mapstructure v0.0.0-20180111000720-b4575eea38cc h1:5T6hzGUO5OrL6MdYXYoLQtRWJDDgjdlOVBn9mIqGY1g=

packer/communicator.go

@@ -4,11 +4,11 @@ import (
 	"context"
 	"io"
 	"os"
+	"strings"
 	"sync"
+	"unicode"
 
-	"golang.org/x/sync/errgroup"
-
-	"github.com/hashicorp/packer/common/iochan"
+	"github.com/mitchellh/iochan"
 )
 
 // CmdDisconnect is a sentinel value to indicate a RemoteCmd
@@ -115,30 +115,52 @@ func (r *RemoteCmd) RunWithUi(ctx context.Context, c Communicator, ui Ui) error
 		r.Stderr = io.MultiWriter(r.Stderr, stderr_w)
 	}
 
-	// Loop and get all our output until done.
-	printFn := func(in io.Reader, out func(string)) error {
-		for output := range iochan.LineReader(in) {
+	// Start the command
+	if err := c.Start(ctx, r); err != nil {
+		return err
+	}
+
+	// Create the channels we'll use for data
+	stdoutCh := iochan.DelimReader(stdout_r, '\n')
+	stderrCh := iochan.DelimReader(stderr_r, '\n')
+
+	// Start the goroutine to watch for the exit
+	go func() {
+		defer stdout_w.Close()
+		defer stderr_w.Close()
+		r.Wait()
+	}()
+
+	// Loop and get all our output
+OutputLoop:
+	for {
+		select {
+		case output := <-stderrCh:
+			if output != "" {
+				ui.Error(r.cleanOutputLine(output))
+			}
+		case output := <-stdoutCh:
 			if output != "" {
-				out(output)
+				ui.Message(r.cleanOutputLine(output))
 			}
+		case <-r.exitCh:
+			break OutputLoop
+		case <-ctx.Done():
+			return ctx.Err()
 		}
-		return nil
 	}
 
-	wg, ctx := errgroup.WithContext(ctx)
-
-	wg.Go(func() error { return printFn(stdout_r, ui.Message) })
-	wg.Go(func() error { return printFn(stderr_r, ui.Error) })
-
-	if err := c.Start(ctx, r); err != nil {
-		return err
+	// Make sure we finish off stdout/stderr because we may have gotten
+	// a message from the exit channel before finishing these first.
+	for output := range stdoutCh {
+		ui.Message(r.cleanOutputLine(output))
 	}
-	select {
-	case <-ctx.Done():
-		return ctx.Err()
-	case <-r.exitCh:
-		return nil
+
+	for output := range stderrCh {
+		ui.Error(r.cleanOutputLine(output))
 	}
+
+	return nil
 }
 
 // SetExited is a helper for setting that this process is exited. This
@@ -174,3 +196,19 @@ func (r *RemoteCmd) initchan() {
 		}
 	})
 }
+
+// cleanOutputLine cleans up a line so that '\r' don't muck up the
+// UI output when we're reading from a remote command.
+func (r *RemoteCmd) cleanOutputLine(line string) string {
+	// Trim surrounding whitespace
+	line = strings.TrimRightFunc(line, unicode.IsSpace)
+
+	// Trim up to the first carriage return, since that text would be
+	// lost anyways.
+	idx := strings.LastIndex(line, "\r")
+	if idx > -1 {
+		line = line[idx+1:]
+	}
+
+	return line
+}

vendor/github.com/mitchellh/iochan/LICENSE.md

@@ -0,0 +1,21 @@
+The MIT License (MIT)
+
+Copyright (c) 2015 Mitchell Hashimoto
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.

vendor/github.com/mitchellh/iochan/README.md

@@ -0,0 +1,13 @@
+# iochan
+
+iochan is a Go library for treating `io` readers and writers like channels.
+This is useful when sometimes you wish to use `io.Reader` and such in `select`
+statements.
+
+## Installation
+
+Standard `go get`:
+
+```
+$ go get github.com/mitchellh/iochan
+```

vendor/github.com/mitchellh/iochan/go.mod

@@ -0,0 +1 @@
+module github.com/mitchellh/iochan

vendor/github.com/mitchellh/iochan/iochan.go

@@ -0,0 +1,41 @@
+package iochan
+
+import (
+	"bufio"
+	"io"
+)
+
+// DelimReader takes an io.Reader and produces the contents of the reader
+// on the returned channel. The contents on the channel will be returned
+// on boundaries specified by the delim parameter, and will include this
+// delimiter.
+//
+// If an error occurs while reading from the reader, the reading will end.
+//
+// In the case of an EOF or error, the channel will be closed.
+//
+// This must only be called once for any individual reader. The behavior is
+// unknown and will be unexpected if this is called multiple times with the
+// same reader.
+func DelimReader(r io.Reader, delim byte) <-chan string {
+	ch := make(chan string)
+
+	go func() {
+		buf := bufio.NewReader(r)
+
+		for {
+			line, err := buf.ReadString(delim)
+			if line != "" {
+				ch <- line
+			}
+
+			if err != nil {
+				break
+			}
+		}
+
+		close(ch)
+	}()
+
+	return ch
+}

vendor/modules.txt

@@ -340,6 +340,8 @@ github.com/mitchellh/go-homedir
 github.com/mitchellh/go-testing-interface
 # github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed
 github.com/mitchellh/go-vnc
+# github.com/mitchellh/iochan v1.0.0
+github.com/mitchellh/iochan
 # github.com/mitchellh/mapstructure v0.0.0-20180111000720-b4575eea38cc
 github.com/mitchellh/mapstructure
 # github.com/mitchellh/panicwrap v0.0.0-20170106182340-fce601fe5557