Merge pull request #1340 from AkihiroSuda/fix-os-race

connhelper: fix cmd.Wait() race
This commit is contained in:
Tibor Vass 2018-09-05 15:59:37 -07:00 committed by GitHub
commit 612673dd01
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 14 additions and 2 deletions

View File

@ -82,6 +82,7 @@ func newCommandConn(ctx context.Context, cmd string, args ...string) (net.Conn,
// commandConn implements net.Conn // commandConn implements net.Conn
type commandConn struct { type commandConn struct {
cmd *exec.Cmd cmd *exec.Cmd
cmdMutex sync.Mutex
stdin io.WriteCloser stdin io.WriteCloser
stdout io.ReadCloser stdout io.ReadCloser
stderrMu sync.Mutex stderrMu sync.Mutex
@ -102,6 +103,7 @@ func (c *commandConn) killIfStdioClosed() error {
return nil return nil
} }
var err error var err error
c.cmdMutex.Lock()
// NOTE: maybe already killed here // NOTE: maybe already killed here
if err = c.cmd.Process.Kill(); err == nil { if err = c.cmd.Process.Kill(); err == nil {
err = c.cmd.Wait() err = c.cmd.Wait()
@ -113,11 +115,14 @@ func (c *commandConn) killIfStdioClosed() error {
err = nil err = nil
} }
} }
c.cmdMutex.Unlock()
return err return err
} }
func (c *commandConn) onEOF(eof error) error { func (c *commandConn) onEOF(eof error) error {
c.cmdMutex.Lock()
werr := c.cmd.Wait() werr := c.cmd.Wait()
c.cmdMutex.Unlock()
if werr == nil { if werr == nil {
return eof return eof
} }
@ -131,6 +136,7 @@ func ignorableCloseError(err error) bool {
errS := err.Error() errS := err.Error()
ss := []string{ ss := []string{
os.ErrClosed.Error(), os.ErrClosed.Error(),
"process already finished",
} }
for _, s := range ss { for _, s := range ss {
if strings.Contains(errS, s) { if strings.Contains(errS, s) {
@ -148,7 +154,10 @@ func (c *commandConn) CloseRead() error {
c.stdioClosedMu.Lock() c.stdioClosedMu.Lock()
c.stdoutClosed = true c.stdoutClosed = true
c.stdioClosedMu.Unlock() c.stdioClosedMu.Unlock()
return c.killIfStdioClosed() if err := c.killIfStdioClosed(); err != nil && !ignorableCloseError(err) {
logrus.Warnf("commandConn.CloseRead: %v", err)
}
return nil
} }
func (c *commandConn) Read(p []byte) (int, error) { func (c *commandConn) Read(p []byte) (int, error) {
@ -167,7 +176,10 @@ func (c *commandConn) CloseWrite() error {
c.stdioClosedMu.Lock() c.stdioClosedMu.Lock()
c.stdinClosed = true c.stdinClosed = true
c.stdioClosedMu.Unlock() c.stdioClosedMu.Unlock()
return c.killIfStdioClosed() if err := c.killIfStdioClosed(); err != nil && !ignorableCloseError(err) {
logrus.Warnf("commandConn.CloseWrite: %v", err)
}
return nil
} }
func (c *commandConn) Write(p []byte) (int, error) { func (c *commandConn) Write(p []byte) (int, error) {