diff --git a/process.go b/process.go index cc978e5dc..33bd24594 100644 --- a/process.go +++ b/process.go @@ -3,8 +3,8 @@ package actionlint import ( "context" "fmt" - "io" "os/exec" + "strings" "sync" "github.com/mattn/go-shellwords" @@ -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 { diff --git a/process_test.go b/process_test.go index 0ba29bd1a..04f1afc14 100644 --- a/process_test.go +++ b/process_test.go @@ -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{