aboutsummaryrefslogtreecommitdiff
path: root/src/os/exec
diff options
context:
space:
mode:
authorBryan C. Mills <bcmills@google.com>2023-06-30 11:50:47 -0400
committerGopher Robot <gobot@golang.org>2023-07-10 19:19:59 +0000
commit07ede7a54379eef959cf29af5a87ea19c78a31fd (patch)
treec5492e88cd6f4532628853acc2c764f855a2a0bb /src/os/exec
parent7dc62f3bda96359cc1904f4ea387f9a1c82c9f9d (diff)
downloadgo-07ede7a54379eef959cf29af5a87ea19c78a31fd.tar.xz
syscall: serialize locks on ForkLock on platforms where forkExecPipe is not atomic
In CL 421441, we changed syscall to allow concurrent calls to forkExec. On platforms that support the pipe2 syscall that is the right behavior, because pipe2 atomically opens the pipe with CLOEXEC already set. However, on platforms that do not support pipe2 (currently aix and darwin), syscall.forkExecPipe is not atomic, and the pipes do not initially have CLOEXEC set. If two calls to forkExec proceed concurrently, a pipe intended for one child process can be accidentally inherited by the other. If the process is long-lived, the pipe can be held open unexpectedly and prevent the parent process from reaching EOF reading the child's status from the pipe. Fixes #61080. Updates #23558. Updates #54162. Change-Id: I83edcc80674ff267a39d06260c5697c654ff5a4b Reviewed-on: https://go-review.googlesource.com/c/go/+/507355 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
Diffstat (limited to 'src/os/exec')
-rw-r--r--src/os/exec/exec_test.go72
1 files changed, 72 insertions, 0 deletions
diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go
index 67cd446f42..d37fffd39d 100644
--- a/src/os/exec/exec_test.go
+++ b/src/os/exec/exec_test.go
@@ -1708,3 +1708,75 @@ func TestCancelErrors(t *testing.T) {
}
})
}
+
+// TestConcurrentExec is a regression test for https://go.dev/issue/61080.
+//
+// Forking multiple child processes concurrently would sometimes hang on darwin.
+// (This test hung on a gomote with -count=100 after only a few iterations.)
+func TestConcurrentExec(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.Background())
+
+ // This test will spawn nHangs subprocesses that hang reading from stdin,
+ // and nExits subprocesses that exit immediately.
+ //
+ // When issue #61080 was present, a long-lived "hang" subprocess would
+ // occasionally inherit the fork/exec status pipe from an "exit" subprocess,
+ // causing the parent process (which expects to see an EOF on that pipe almost
+ // immediately) to unexpectedly block on reading from the pipe.
+ var (
+ nHangs = runtime.GOMAXPROCS(0)
+ nExits = runtime.GOMAXPROCS(0)
+ hangs, exits sync.WaitGroup
+ )
+ hangs.Add(nHangs)
+ exits.Add(nExits)
+
+ // ready is done when the goroutines have done as much work as possible to
+ // prepare to create subprocesses. It isn't strictly necessary for the test,
+ // but helps to increase the repro rate by making it more likely that calls to
+ // syscall.StartProcess for the "hang" and "exit" goroutines overlap.
+ var ready sync.WaitGroup
+ ready.Add(nHangs + nExits)
+
+ for i := 0; i < nHangs; i++ {
+ go func() {
+ defer hangs.Done()
+
+ cmd := helperCommandContext(t, ctx, "pipetest")
+ stdin, err := cmd.StdinPipe()
+ if err != nil {
+ ready.Done()
+ t.Error(err)
+ return
+ }
+ cmd.Cancel = stdin.Close
+ ready.Done()
+
+ ready.Wait()
+ if err := cmd.Start(); err != nil {
+ t.Error(err)
+ return
+ }
+
+ cmd.Wait()
+ }()
+ }
+
+ for i := 0; i < nExits; i++ {
+ go func() {
+ defer exits.Done()
+
+ cmd := helperCommandContext(t, ctx, "exit", "0")
+ ready.Done()
+
+ ready.Wait()
+ if err := cmd.Run(); err != nil {
+ t.Error(err)
+ }
+ }()
+ }
+
+ exits.Wait()
+ cancel()
+ hangs.Wait()
+}