Merge pull request #651 from attehuhtakangas/fix-shellcheck-deadlock-macos

fix deadlock in shellcheck integration on darwin by using cmd.Stdin

Author: rhysd

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 {

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{