Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions process.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package actionlint
import (
"context"
"fmt"
"io"
"os/exec"
"strings"
"sync"

"github.com/mattn/go-shellwords"
Expand All @@ -24,18 +24,14 @@ type cmdExecution struct {
func (e *cmdExecution) run() ([]byte, error) {
cmd := exec.Command(e.cmd, e.args...)
cmd.Stderr = nil

p, err := cmd.StdinPipe()
if err != nil {
return nil, fmt.Errorf("could not make stdin pipe for %s process: %w", e.cmd, err)
}
if _, err := io.WriteString(p, e.stdin); err != nil {
p.Close()
return nil, fmt.Errorf("could not write to stdin of %s process: %w", e.cmd, err)
}
p.Close()
// Set stdin via an io.Reader so that exec.Cmd pipes the bytes to the child
// after Start(). Writing to cmd.StdinPipe() before Start() relies on the
// kernel pipe buffer being large enough to absorb the whole payload, which
// deadlocks on darwin when multiple workers run concurrently (issue #650).
cmd.Stdin = strings.NewReader(e.stdin)

var stdout []byte
var err error
if e.combineOutput {
stdout, err = cmd.CombinedOutput()
} else {
Expand Down
43 changes: 43 additions & 0 deletions process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,49 @@ func TestProcessInputStdin(t *testing.T) {
}
}

// Regression test for issue #650: concurrent runs with a stdin payload larger
// than the kernel pipe buffer used to deadlock on darwin because the payload
// was written to cmd.StdinPipe() before cmd.Start().
func TestProcessConcurrentStdinDoesNotDeadlock(t *testing.T) {
p := newConcurrentProcess(5)

// 64 KiB is above the default pipe buffer size on darwin and Linux so it
// forces the stdin copy to happen after the child has started.
payload := strings.Repeat("x", 64*1024)

done := make(chan struct{})
go func() {
defer close(done)
cmds := make([]*externalCommand, 0, 5)
for i := 0; i < 5; i++ {
cat := testSkipIfNoCommand(t, p, "cat")
cat.run(nil, payload, func(b []byte, err error) error {
if err != nil {
t.Errorf("cat failed: %v", err)
return err
}
if len(b) != len(payload) {
t.Errorf("cat output length %d, want %d", len(b), len(payload))
}
return nil
})
cmds = append(cmds, cat)
}
for _, c := range cmds {
if err := c.wait(); err != nil {
t.Errorf("cat wait failed: %v", err)
}
}
p.wait()
}()

select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatal("concurrent stdin writes deadlocked — see issue #650")
}
}

func TestProcessErrorCommandNotFound(t *testing.T) {
p := newConcurrentProcess(1)
c := &externalCommand{
Expand Down
Loading