aboutsummaryrefslogtreecommitdiffstats
path: root/plumbing/transport
diff options
context:
space:
mode:
authorMáximo Cuadros <mcuadros@gmail.com>2017-07-07 16:39:06 -0700
committerGitHub <noreply@github.com>2017-07-07 16:39:06 -0700
commit6b69a1630b30c41f4563fd95aca1d647ba611adf (patch)
tree342bfb394fdd7b22a4bc7e3deaf2f32c73585f30 /plumbing/transport
parent83c0e738df4b676e6c4aeed5fd1bc7c90fb341c5 (diff)
parent6506d377f7dc70b72d7454f5901d8199023dd8ed (diff)
downloadgo-git-6b69a1630b30c41f4563fd95aca1d647ba611adf.tar.gz
Merge pull request #464 from smola/race-463
transport/file: avoid race with Command.Wait, fixes #463
Diffstat (limited to 'plumbing/transport')
-rw-r--r--plumbing/transport/file/client.go18
-rw-r--r--plumbing/transport/internal/common/common.go34
2 files changed, 31 insertions, 21 deletions
diff --git a/plumbing/transport/file/client.go b/plumbing/transport/file/client.go
index a199b01..b6d60c1 100644
--- a/plumbing/transport/file/client.go
+++ b/plumbing/transport/file/client.go
@@ -46,8 +46,9 @@ func (r *runner) Command(cmd string, ep transport.Endpoint, auth transport.AuthM
}
type command struct {
- cmd *exec.Cmd
- closed bool
+ cmd *exec.Cmd
+ stderrCloser io.Closer
+ closed bool
}
func (c *command) Start() error {
@@ -55,7 +56,12 @@ func (c *command) Start() error {
}
func (c *command) StderrPipe() (io.Reader, error) {
- return c.cmd.StderrPipe()
+ // Pipe returned by Command.StderrPipe has a race with Read + Command.Wait.
+ // We use an io.Pipe and close it after the command finishes.
+ r, w := io.Pipe()
+ c.cmd.Stderr = w
+ c.stderrCloser = r
+ return r, nil
}
func (c *command) StdinPipe() (io.WriteCloser, error) {
@@ -72,7 +78,11 @@ func (c *command) Close() error {
return nil
}
- defer func() { c.closed = true }()
+ defer func() {
+ c.closed = true
+ _ = c.stderrCloser.Close()
+ }()
+
err := c.cmd.Wait()
if _, ok := err.(*os.PathError); ok {
return nil
diff --git a/plumbing/transport/internal/common/common.go b/plumbing/transport/internal/common/common.go
index c1e1518..04db770 100644
--- a/plumbing/transport/internal/common/common.go
+++ b/plumbing/transport/internal/common/common.go
@@ -10,6 +10,7 @@ import (
"errors"
"fmt"
"io"
+ stdioutil "io/ioutil"
"strings"
"time"
@@ -22,7 +23,6 @@ import (
const (
readErrorSecondsTimeout = 10
- errLinesBuffer = 1000
)
var (
@@ -96,7 +96,7 @@ type session struct {
advRefs *packp.AdvRefs
packRun bool
finished bool
- errLines chan string
+ firstErrLine chan string
}
func (c *client) newSession(s string, ep transport.Endpoint, auth transport.AuthMethod) (*session, error) {
@@ -128,26 +128,29 @@ func (c *client) newSession(s string, ep transport.Endpoint, auth transport.Auth
Stdin: stdin,
Stdout: stdout,
Command: cmd,
- errLines: c.listenErrors(stderr),
+ firstErrLine: c.listenFirstError(stderr),
isReceivePack: s == transport.ReceivePackServiceName,
}, nil
}
-func (c *client) listenErrors(r io.Reader) chan string {
+func (c *client) listenFirstError(r io.Reader) chan string {
if r == nil {
return nil
}
- errLines := make(chan string, errLinesBuffer)
+ errLine := make(chan string, 1)
go func() {
s := bufio.NewScanner(r)
- for s.Scan() {
- line := string(s.Bytes())
- errLines <- line
+ if s.Scan() {
+ errLine <- s.Text()
+ } else {
+ close(errLine)
}
+
+ _, _ = io.Copy(stdioutil.Discard, r)
}()
- return errLines
+ return errLine
}
// AdvertisedReferences retrieves the advertised references from the server.
@@ -296,13 +299,10 @@ func (s *session) finish() error {
return nil
}
-func (s *session) Close() error {
- if err := s.finish(); err != nil {
- _ = s.Command.Close()
- return err
- }
-
- return s.Command.Close()
+func (s *session) Close() (err error) {
+ defer ioutil.CheckClose(s.Command, &err)
+ err = s.finish()
+ return
}
func (s *session) checkNotFoundError() error {
@@ -312,7 +312,7 @@ func (s *session) checkNotFoundError() error {
select {
case <-t.C:
return ErrTimeoutExceeded
- case line, ok := <-s.errLines:
+ case line, ok := <-s.firstErrLine:
if !ok {
return nil
}