[go] syscall: wait for all child processes when running as PID 1
254 views
Skip to first unread message
Gerrit Bot (Gerrit)
Jul 10, 2023, 12:59:50 PM7/10/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Gerrit Bot has uploaded this change for review.
syscall: wait for all child processes when running as PID 1
When running a Go binary on a UNIX system, the model for launching processes is fairly simple:
- You first call syscall.StartProcess(), which does a fork under the hood. This returns the PID.
- You call syscall.Wait4(pid, [...]) to obtain the execution results.
os.Process.Wait() extends upon this model a bit, where instead of calling syscall.Wait4() directly, it first calls waitid()/wait6() with WNOWAIT. This allows it to first complete all calls to os.Process.Kill() before destroying the process. This guarantees that signals are not sent to the wrong process when PIDs are recycled quickly.
The model above is perfect for the case where Go binaries are run as regular user processes. One major advantage is that it composes fairly well. Individual goroutines can launch subprocesses without interfering with each other. It may even work well if subprocesses are launched through cgo, assuming syscall.ForkLock is acquired.
Unfortunately, this model is less ideal for environments where Go binaries are run as PID 1 (e.g., inside containers or on embedded devices). In those cases (great-)grandchildren may be reparented towards PID 1 if their parents terminate before they do. PID 1 does not get explicitly notified if such reparenting occurs. This means that PID 1 must use a different model, where it continously calls syscall.Wait4(-1) to get notified of any terminated child process. This does not play nicely with other parts of a program calling syscall.Wait4(pid), as those may see their wait results being stolen.
This commit changes the behavior of os.StartProcess() to use a process-wide goroutine that calls syscall.Wait4(-1) and delivers the wait results to the right instance of os.Process through channels. Launching this goroutine is done through syscall.StartProcess(). Delivery of wait results is performed though a ProcessHandle, which is returned through the 'handle' return value of this function. This return value has thus far only been used on Windows.
Care has been taken to only change the system call behavior when a Go binary runs as PID 1. When running as any other PID, syscall.StartProcess() will return a ProcessHandle that calls syscall.Wait4(pid). This is done for two reasons:
- It ensures that code that launches through alternative means (e.g., cgo) are unaffected.
- Existing code that ignores the 'handle' return value can safely drop the handle and call syscall.Wait4() manually.
The goroutine that we launch also terminates when all known child processes launched through syscall.StartProcess() have terminated. This means that for code running as PID 1, it still remains safe to launch processes through alternative means, as long as it's not done at moments at which one or more processes launched through syscall.StartProcess() are running. syscall.ForkExec() is unaffected.
As the PID of the subprocess is only known after forking, there may be a race condition where registration of the subprocess is only done after the subprocess terminates. This could cause delivery of the wait results to fail. To prevent this from happening, the map of process handles used by the goroutine is protected using the ForkLock. As syscall.StartProcess() nowadays only acquires ForkLock for reading, we use an auxiliary lock to prevent a data race if syscall.StartProcess() is called in parallel.
Change-Id: Id899ca7dc2bad4ccb541f5975e353a0805484cae
GitHub-Last-Rev: 197c286ef4b83347b71c50e57ffb5b4cebc34e8a
GitHub-Pull-Request: golang/go#61261
---
M src/os/exec.go
M src/os/exec_unix.go
D src/os/wait6_dragonfly.go
D src/os/wait6_freebsd64.go
D src/os/wait6_freebsd_386.go
D src/os/wait6_freebsd_arm.go
D src/os/wait6_netbsd.go
D src/os/wait_wait6.go
D src/os/wait_waitid.go
M src/syscall/exec_plan9.go
M src/syscall/exec_unix.go
M src/syscall/exec_windows.go
A src/syscall/wait6_dragonfly.go
A src/syscall/wait6_freebsd64.go
A src/syscall/wait6_freebsd_386.go
A src/syscall/wait6_freebsd_arm.go
A src/syscall/wait6_netbsd.go
R src/syscall/wait_unimp.go
A src/syscall/wait_wait6.go
A src/syscall/wait_waitid.go
20 files changed, 417 insertions(+), 250 deletions(-)
diff --git a/src/os/exec.go b/src/os/exec.go
index ed5a75c..4efbd36 100644
--- a/src/os/exec.go
+++ b/src/os/exec.go
@@ -8,8 +8,6 @@
"errors"
"internal/testlog"
"runtime"
- "sync"
- "sync/atomic"
"syscall"
"time"
)
@@ -20,25 +18,15 @@
// Process stores the information about a process created by StartProcess.
type Process struct {
Pid int
- handle uintptr // handle is accessed atomically on Windows
- isdone atomic.Bool // process has been successfully waited on
- sigMu sync.RWMutex // avoid race between wait and signal
+ handle syscall.ProcessHandle // handle is accessed atomically on Windows
}
-func newProcess(pid int, handle uintptr) *Process {
+func newProcess(pid int, handle syscall.ProcessHandle) *Process {
p := &Process{Pid: pid, handle: handle}
runtime.SetFinalizer(p, (*Process).Release)
return p
}
-func (p *Process) setDone() {
- p.isdone.Store(true)
-}
-
-func (p *Process) done() bool {
- return p.isdone.Load()
-}
-
// ProcAttr holds the attributes that will be applied to a new process
// started by StartProcess.
type ProcAttr struct {
diff --git a/src/os/exec_unix.go b/src/os/exec_unix.go
index f9063b4..17f8739 100644
--- a/src/os/exec_unix.go
+++ b/src/os/exec_unix.go
@@ -14,45 +14,18 @@
)
func (p *Process) wait() (ps *ProcessState, err error) {
- if p.Pid == -1 {
+ if p.handle == nil {
return nil, syscall.EINVAL
}
-
- // If we can block until Wait4 will succeed immediately, do so.
- ready, err := p.blockUntilWaitable()
- if err != nil {
- return nil, err
- }
- if ready {
- // Mark the process done now, before the call to Wait4,
- // so that Process.signal will not send a signal.
- p.setDone()
- // Acquire a write lock on sigMu to wait for any
- // active call to the signal method to complete.
- p.sigMu.Lock()
- p.sigMu.Unlock()
- }
-
var (
status syscall.WaitStatus
rusage syscall.Rusage
- pid1 int
- e error
)
- for {
- pid1, e = syscall.Wait4(p.Pid, &status, 0, &rusage)
- if e != syscall.EINTR {
- break
- }
- }
- if e != nil {
+ if e := p.handle.Wait4(&status, &rusage); e != nil {
return nil, NewSyscallError("wait", e)
}
- if pid1 != 0 {
- p.setDone()
- }
ps = &ProcessState{
- pid: pid1,
+ pid: p.Pid,
status: status,
rusage: &rusage,
}
@@ -66,10 +39,11 @@
if p.Pid == 0 {
return errors.New("os: process not initialized")
}
- p.sigMu.RLock()
- defer p.sigMu.RUnlock()
- if p.done() {
- return ErrProcessDone
+ if p.handle != nil {
+ if !p.handle.Hold() {
+ return ErrProcessDone
+ }
+ defer p.handle.Unhold()
}
s, ok := sig.(syscall.Signal)
if !ok {
@@ -87,6 +61,7 @@
func (p *Process) release() error {
// NOOP for unix.
p.Pid = -1
+ p.handle = nil
// no need for a finalizer anymore
runtime.SetFinalizer(p, nil)
return nil
@@ -94,7 +69,7 @@
func findProcess(pid int) (p *Process, err error) {
// NOOP for unix.
- return newProcess(pid, 0), nil
+ return newProcess(pid, nil), nil
}
func (p *ProcessState) userTime() time.Duration {
diff --git a/src/os/wait6_dragonfly.go b/src/os/wait6_dragonfly.go
deleted file mode 100644
index cc3af39..0000000
--- a/src/os/wait6_dragonfly.go
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2022 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package os
-
-import (
- "syscall"
- "unsafe"
-)
-
-const _P_PID = 0
-
-func wait6(idtype, id, options int) (status int, errno syscall.Errno) {
- var status32 int32 // C.int
- _, _, errno = syscall.Syscall6(syscall.SYS_WAIT6, uintptr(idtype), uintptr(id), uintptr(unsafe.Pointer(&status32)), uintptr(options), 0, 0)
- return int(status32), errno
-}
diff --git a/src/os/wait6_freebsd64.go b/src/os/wait6_freebsd64.go
deleted file mode 100644
index b2677c5..0000000
--- a/src/os/wait6_freebsd64.go
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright 2022 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build freebsd && (amd64 || arm64 || riscv64)
-
-package os
-
-import (
- "syscall"
- "unsafe"
-)
-
-const _P_PID = 0
-
-func wait6(idtype, id, options int) (status int, errno syscall.Errno) {
- var status32 int32 // C.int
- _, _, errno = syscall.Syscall6(syscall.SYS_WAIT6, uintptr(idtype), uintptr(id), uintptr(unsafe.Pointer(&status32)), uintptr(options), 0, 0)
- return int(status32), errno
-}
diff --git a/src/os/wait6_freebsd_386.go b/src/os/wait6_freebsd_386.go
deleted file mode 100644
index 30b01c5..0000000
--- a/src/os/wait6_freebsd_386.go
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2022 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package os
-
-import (
- "syscall"
- "unsafe"
-)
-
-const _P_PID = 0
-
-func wait6(idtype, id, options int) (status int, errno syscall.Errno) {
- // freebsd32_wait6_args{ idtype, id1, id2, status, options, wrusage, info }
- _, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, uintptr(idtype), uintptr(id), 0, uintptr(unsafe.Pointer(&status)), uintptr(options), 0, 0, 0, 0)
- return status, errno
-}
diff --git a/src/os/wait6_freebsd_arm.go b/src/os/wait6_freebsd_arm.go
deleted file mode 100644
index 0fd8af0..0000000
--- a/src/os/wait6_freebsd_arm.go
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2022 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package os
-
-import (
- "syscall"
- "unsafe"
-)
-
-const _P_PID = 0
-
-func wait6(idtype, id, options int) (status int, errno syscall.Errno) {
- // freebsd32_wait6_args{ idtype, pad, id1, id2, status, options, wrusage, info }
- _, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, uintptr(idtype), 0, uintptr(id), 0, uintptr(unsafe.Pointer(&status)), uintptr(options), 0, 0, 0)
- return status, errno
-}
diff --git a/src/os/wait6_netbsd.go b/src/os/wait6_netbsd.go
deleted file mode 100644
index 0bbb73d..0000000
--- a/src/os/wait6_netbsd.go
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2022 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package os
-
-import (
- "syscall"
- "unsafe"
-)
-
-const _P_PID = 1 // not 0 as on FreeBSD and Dragonfly!
-
-func wait6(idtype, id, options int) (status int, errno syscall.Errno) {
- var status32 int32 // C.int
- _, _, errno = syscall.Syscall6(syscall.SYS_WAIT6, uintptr(idtype), uintptr(id), uintptr(unsafe.Pointer(&status32)), uintptr(options), 0, 0)
- return int(status32), errno
-}
diff --git a/src/os/wait_wait6.go b/src/os/wait_wait6.go
deleted file mode 100644
index 1031428..0000000
--- a/src/os/wait_wait6.go
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright 2016 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build dragonfly || freebsd || netbsd
-
-package os
-
-import (
- "runtime"
- "syscall"
-)
-
-// blockUntilWaitable attempts to block until a call to p.Wait will
-// succeed immediately, and reports whether it has done so.
-// It does not actually call p.Wait.
-func (p *Process) blockUntilWaitable() (bool, error) {
- var errno syscall.Errno
- for {
- _, errno = wait6(_P_PID, p.Pid, syscall.WEXITED|syscall.WNOWAIT)
- if errno != syscall.EINTR {
- break
- }
- }
- runtime.KeepAlive(p)
- if errno == syscall.ENOSYS {
- return false, nil
- } else if errno != 0 {
- return false, NewSyscallError("wait6", errno)
- }
- return true, nil
-}
diff --git a/src/os/wait_waitid.go b/src/os/wait_waitid.go
deleted file mode 100644
index c0503b2..0000000
--- a/src/os/wait_waitid.go
+++ /dev/null
@@ -1,48 +0,0 @@
-// Copyright 2016 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// We used to used this code for Darwin, but according to issue #19314
-// waitid returns if the process is stopped, even when using WEXITED.
-
-//go:build linux
-
-package os
-
-import (
- "runtime"
- "syscall"
- "unsafe"
-)
-
-const _P_PID = 1
-
-// blockUntilWaitable attempts to block until a call to p.Wait will
-// succeed immediately, and reports whether it has done so.
-// It does not actually call p.Wait.
-func (p *Process) blockUntilWaitable() (bool, error) {
- // The waitid system call expects a pointer to a siginfo_t,
- // which is 128 bytes on all Linux systems.
- // On darwin/amd64, it requires 104 bytes.
- // We don't care about the values it returns.
- var siginfo [16]uint64
- psig := &siginfo[0]
- var e syscall.Errno
- for {
- _, _, e = syscall.Syscall6(syscall.SYS_WAITID, _P_PID, uintptr(p.Pid), uintptr(unsafe.Pointer(psig)), syscall.WEXITED|syscall.WNOWAIT, 0, 0)
- if e != syscall.EINTR {
- break
- }
- }
- runtime.KeepAlive(p)
- if e != 0 {
- // waitid has been available since Linux 2.6.9, but
- // reportedly is not available in Ubuntu on Windows.
- // See issue 16610.
- if e == syscall.ENOSYS {
- return false, nil
- }
- return false, NewSyscallError("waitid", e)
- }
- return true, nil
-}
diff --git a/src/syscall/exec_plan9.go b/src/syscall/exec_plan9.go
index 8762237..3f51a38 100644
--- a/src/syscall/exec_plan9.go
+++ b/src/syscall/exec_plan9.go
@@ -528,8 +528,10 @@
return startProcess(argv0, argv, attr)
}
+type ProcessHandle = uintptr
+
// StartProcess wraps ForkExec for package os.
-func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle uintptr, err error) {
+func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle ProcessHandle, err error) {
pid, err = startProcess(argv0, argv, attr)
return pid, 0, err
}
diff --git a/src/syscall/exec_unix.go b/src/syscall/exec_unix.go
index 14edd02..50aa1f0 100644
--- a/src/syscall/exec_unix.go
+++ b/src/syscall/exec_unix.go
@@ -13,6 +13,7 @@
"internal/bytealg"
"runtime"
"sync"
+ "sync/atomic"
"unsafe"
)
@@ -137,7 +138,150 @@
var zeroProcAttr ProcAttr
var zeroSysProcAttr SysProcAttr
-func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error) {
+type baseProcessHandle struct {
+ terminated atomic.Bool
+ holdMu sync.RWMutex
+}
+
+func (ph *baseProcessHandle) markTerminated(graceful bool) {
+ ph.terminated.Store(true)
+ if graceful {
+ // Acquire a write lock on holdMu to wait for any tasks
+ // that hold the process (e.g., signalling) complete.
+ ph.holdMu.Lock()
+ ph.holdMu.Unlock()
+ }
+}
+
+func (ph *baseProcessHandle) Hold() bool {
+ ph.holdMu.RLock()
+ if ph.terminated.Load() {
+ ph.holdMu.RUnlock()
+ return false
+ }
+ return true
+}
+
+func (ph *baseProcessHandle) Unhold() {
+ ph.holdMu.RUnlock()
+}
+
+// managedProcessHandle is created for processes for which Wait4()
+// should not be called directly. Instead, wait results will be
+// delivered by a dedicated goroutine that calls Wait4(-1). This is
+// necessary to ensure that all zombie processes are removed when
+// running as PID 1.
+type managedProcessHandle struct {
+ baseProcessHandle
+
+ haveStatus chan struct{}
+ err error
+ status WaitStatus
+ rusage Rusage
+}
+
+func (ph *managedProcessHandle) Wait4(status *WaitStatus, rusage *Rusage) error {
+ <-ph.haveStatus
+ if ph.err != nil {
+ return ph.err
+ }
+ *status = ph.status
+ if rusage != nil {
+ *rusage = ph.rusage
+ }
+ return nil
+}
+
+func stopProcessReaper() bool {
+ ForkLock.Lock()
+ defer ForkLock.Unlock()
+ if len(processHandles) > 0 {
+ // forkExec() has been called after the last call to
+ // Wait4(). Continue reaping processes.
+ return false
+ }
+ processReaperRunning = false
+ return true
+}
+
+func reapProcesses() {
+ for {
+ switch pid, err := blockUntilWaitable(0); err {
+ case nil:
+ if pid == 0 {
+ // This operating system does not support
+ // wait6(-1, WNOWAIT) or waitid(-1, WNOWAIT).
+ // We must thus call Wait4(-1, 0). This
+ // unfortunately means we can't delay waiting
+ // until all calls to ProcessHandle.Hold() and
+ // Unhold() have completed.
+ var status WaitStatus
+ var rusage Rusage
+ switch pid, err := Wait4(-1, &status, 0, &rusage); err {
+ case nil:
+ ForkLock.Lock()
+ if ph, ok := processHandles[pid]; ok {
+ delete(processHandles, pid)
+ ph.markTerminated(false)
+ ph.status = status
+ ph.rusage = rusage
+ close(ph.haveStatus)
+ }
+ ForkLock.Unlock()
+ case ECHILD:
+ if stopProcessReaper() {
+ return
+ }
+ case EINTR:
+ default:
+ panic(err)
+ }
+ } else {
+ ForkLock.Lock()
+ if ph, ok := processHandles[pid]; ok {
+ // Process for which we have a valid
+ // handle has terminated.
+ delete(processHandles, pid)
+ ForkLock.Unlock()
+ ph.markTerminated(true)
+ for {
+ if _, ph.err = Wait4(pid, &ph.status, 0, &ph.rusage); ph.err != EINTR {
+ break
+ }
+ }
+ close(ph.haveStatus)
+ } else {
+ // Process that has been reparented
+ // to us has terminated. Discard the
+ // wait results.
+ ForkLock.Unlock()
+ var status WaitStatus
+ if _, err := Wait4(pid, &status, 0, nil); err != nil && err != ECHILD && err != EINTR {
+ panic(err)
+ }
+ }
+ }
+ case ECHILD:
+ if stopProcessReaper() {
+ return
+ }
+ default:
+ panic(err)
+ }
+ }
+}
+
+var (
+ processReaperInsertionLock sync.Mutex
+
+ // These fields are locked either by acquiring ForkLock for
+ // writing, or by acquiring ForkLock for reading and
+ // processReaperInsertionLock.
+ processReaperRunning = false
+ processHandles = map[int]*managedProcessHandle{}
+)
+
+func forkExec(argv0 string, argv []string, attr *ProcAttr, waitInBackground bool) (pid int, handle *managedProcessHandle, err error) {
var p [2]int
var n int
var err1 Errno
@@ -154,15 +298,15 @@
// Convert args to C form.
argv0p, err := BytePtrFromString(argv0)
if err != nil {
- return 0, err
+ return 0, nil, err
}
argvp, err := SlicePtrFromStrings(argv)
if err != nil {
- return 0, err
+ return 0, nil, err
}
envvp, err := SlicePtrFromStrings(attr.Env)
if err != nil {
- return 0, err
+ return 0, nil, err
}
if (runtime.GOOS == "freebsd" || runtime.GOOS == "dragonfly") && len(argv) > 0 && len(argv[0]) > len(argv0) {
@@ -173,24 +317,24 @@
if sys.Chroot != "" {
chroot, err = BytePtrFromString(sys.Chroot)
if err != nil {
- return 0, err
+ return 0, nil, err
}
}
var dir *byte
if attr.Dir != "" {
dir, err = BytePtrFromString(attr.Dir)
if err != nil {
- return 0, err
+ return 0, nil, err
}
}
// Both Setctty and Foreground use the Ctty field,
// but they give it slightly different meanings.
if sys.Setctty && sys.Foreground {
- return 0, errorspkg.New("both Setctty and Foreground set in SysProcAttr")
+ return 0, nil, errorspkg.New("both Setctty and Foreground set in SysProcAttr")
}
if sys.Setctty && sys.Ctty >= len(attr.Files) {
- return 0, errorspkg.New("Setctty set but Ctty not valid in child")
+ return 0, nil, errorspkg.New("Setctty set but Ctty not valid in child")
}
acquireForkLock()
@@ -198,7 +342,7 @@
// Allocate child status pipe close on exec.
if err = forkExecPipe(p[:]); err != nil {
releaseForkLock()
- return 0, err
+ return 0, nil, err
}
// Kick off child.
@@ -207,7 +351,24 @@
Close(p[0])
Close(p[1])
releaseForkLock()
- return 0, Errno(err1)
+ return 0, nil, Errno(err1)
+ }
+
+ if waitInBackground {
+ processReaperInsertionLock.Lock()
+ handle = &managedProcessHandle{
+ haveStatus: make(chan struct{}),
+ }
+ if _, ok := processHandles[pid]; ok {
+ panic("Process ID has been recycled before wait status was obtained")
+ }
+ processHandles[pid] = handle
+
+ if !processReaperRunning {
+ processReaperRunning = true
+ go reapProcesses()
+ }
+ processReaperInsertionLock.Unlock()
}
releaseForkLock()
@@ -228,17 +389,19 @@
err = EPIPE
}
- // Child failed; wait for it to exit, to make sure
- // the zombies don't accumulate.
- _, err1 := Wait4(pid, &wstatus, 0, nil)
- for err1 == EINTR {
- _, err1 = Wait4(pid, &wstatus, 0, nil)
+ if !waitInBackground {
+ // Child failed; wait for it to exit, to make sure
+ // the zombies don't accumulate.
+ _, err1 := Wait4(pid, &wstatus, 0, nil)
+ for err1 == EINTR {
+ _, err1 = Wait4(pid, &wstatus, 0, nil)
+ }
}
- return 0, err
+ return 0, nil, err
}
// Read got EOF, so pipe closed on exec, so exec succeeded.
- return pid, nil
+ return pid, handle, nil
}
var (
@@ -325,14 +488,67 @@
}
// Combination of fork and exec, careful to be thread safe.
-func ForkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error) {
- return forkExec(argv0, argv, attr)
+func ForkExec(argv0 string, argv []string, attr *ProcAttr) (int, error) {
+ pid, _, err := forkExec(argv0, argv, attr, false)
+ return pid, err
}
+type ProcessHandle interface {
+ Hold() (success bool)
+ Unhold()
+ Wait4(status *WaitStatus, rusage *Rusage) error
+}
+
+// freestandingProcessHandle is created for processes for which
+// Wait4(pid) may be called directly. This is safe if the current
+// process does not have PID 1, as we know no other processes will be
+// reparented to it. There is thus no need to call Wait4(-1).
+type freestandingProcessHandle struct {
+ baseProcessHandle
+ pid int
+}
+
+func (ph *freestandingProcessHandle) Wait4(status *WaitStatus, rusage *Rusage) error {
+ // If we can block until Wait4 will succeed immediately, do so.
+ pid, err := blockUntilWaitable(ph.pid)
+ if err != nil {
+ return err
+ }
+ if pid != 0 {
+ ph.markTerminated(true)
+ }
+ for {
+ if _, err := Wait4(ph.pid, status, 0, rusage); err == nil {
+ ph.markTerminated(false)
+ return nil
+ } else if err != EINTR {
+ return err
+ }
+ }
+}
+
+var (
+ isPID1 bool
+ isPID1Init sync.Once
+)
+
// StartProcess wraps ForkExec for package os.
-func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle uintptr, err error) {
- pid, err = forkExec(argv0, argv, attr)
- return pid, 0, err
+func StartProcess(argv0 string, argv []string, attr *ProcAttr) (int, ProcessHandle, error) {
+ isPID1Init.Do(func() { isPID1 = Getpid() == 1 })
+ if isPID1 {
+ // When running as PID 1, orphan processes may be
+ // reparented to us. This means that calling Wait4(pid)
+ // on is not sufficient to get rid of all zombie
+ // processes. Spawn a dedicated goroutine that
+ // repeatedly calls Wait4(-1) and delivers the
+ return forkExec(argv0, argv, attr, true)
+ }
+
+ pid, _, err := forkExec(argv0, argv, attr, false)
+ if err != nil {
+ return 0, nil, err
+ }
+ return pid, &freestandingProcessHandle{pid: pid}, nil
}
// Implemented in runtime package.
diff --git a/src/syscall/exec_windows.go b/src/syscall/exec_windows.go
index 0a93bc0..52df133 100644
--- a/src/syscall/exec_windows.go
+++ b/src/syscall/exec_windows.go
@@ -255,7 +255,9 @@
var zeroProcAttr ProcAttr
var zeroSysProcAttr SysProcAttr
-func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle uintptr, err error) {
+type ProcessHandle = uintptr
+
+func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle ProcessHandle, err error) {
if len(argv0) == 0 {
return 0, 0, EWINDOWS
}
diff --git a/src/syscall/wait6_dragonfly.go b/src/syscall/wait6_dragonfly.go
new file mode 100644
index 0000000..747908f
--- /dev/null
+++ b/src/syscall/wait6_dragonfly.go
@@ -0,0 +1,17 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package syscall
+
+import (
+ "unsafe"
+)
+
+const _P_PID = 0
+
+func wait6(idtype, id, options int) (status int, errno Errno) {
+ var status32 int32 // C.int
+ _, _, errno = Syscall6(SYS_WAIT6, uintptr(idtype), uintptr(id), uintptr(unsafe.Pointer(&status32)), uintptr(options), 0, 0)
+ return int(status32), errno
+}
diff --git a/src/syscall/wait6_freebsd64.go b/src/syscall/wait6_freebsd64.go
new file mode 100644
index 0000000..4dfabb8
--- /dev/null
+++ b/src/syscall/wait6_freebsd64.go
@@ -0,0 +1,19 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build freebsd && (amd64 || arm64 || riscv64)
+
+package syscall
+
+import (
+ "unsafe"
+)
+
+const _P_PID = 0
+
+func wait6(idtype, id, options int) (status int, errno Errno) {
+ var status32 int32 // C.int
+ _, _, errno = Syscall6(SYS_WAIT6, uintptr(idtype), uintptr(id), uintptr(unsafe.Pointer(&status32)), uintptr(options), 0, 0)
+ return int(status32), errno
+}
diff --git a/src/syscall/wait6_freebsd_386.go b/src/syscall/wait6_freebsd_386.go
new file mode 100644
index 0000000..378cbad
--- /dev/null
+++ b/src/syscall/wait6_freebsd_386.go
@@ -0,0 +1,17 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package syscall
+
+import (
+ "unsafe"
+)
+
+const _P_PID = 0
+
+func wait6(idtype, id, options int) (status int, errno Errno) {
+ // freebsd32_wait6_args{ idtype, id1, id2, status, options, wrusage, info }
+ _, _, errno = Syscall9(SYS_WAIT6, uintptr(idtype), uintptr(id), 0, uintptr(unsafe.Pointer(&status)), uintptr(options), 0, 0, 0, 0)
+ return status, errno
+}
diff --git a/src/syscall/wait6_freebsd_arm.go b/src/syscall/wait6_freebsd_arm.go
new file mode 100644
index 0000000..009c778
--- /dev/null
+++ b/src/syscall/wait6_freebsd_arm.go
@@ -0,0 +1,17 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package syscall
+
+import (
+ "unsafe"
+)
+
+const _P_PID = 0
+
+func wait6(idtype, id, options int) (status int, errno Errno) {
+ // freebsd32_wait6_args{ idtype, pad, id1, id2, status, options, wrusage, info }
+ _, _, errno = Syscall9(SYS_WAIT6, uintptr(idtype), 0, uintptr(id), 0, uintptr(unsafe.Pointer(&status)), uintptr(options), 0, 0, 0)
+ return status, errno
+}
diff --git a/src/syscall/wait6_netbsd.go b/src/syscall/wait6_netbsd.go
new file mode 100644
index 0000000..ef67679
--- /dev/null
+++ b/src/syscall/wait6_netbsd.go
@@ -0,0 +1,17 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package syscall
+
+import (
+ "unsafe"
+)
+
+const _P_PID = 1 // not 0 as on FreeBSD and Dragonfly!
+
+func wait6(idtype, id, options int) (status int, errno Errno) {
+ var status32 int32 // C.int
+ _, _, errno = Syscall6(SYS_WAIT6, uintptr(idtype), uintptr(id), uintptr(unsafe.Pointer(&status32)), uintptr(options), 0, 0)
+ return int(status32), errno
+}
diff --git a/src/os/wait_unimp.go b/src/syscall/wait_unimp.go
similarity index 76%
rename from src/os/wait_unimp.go
rename to src/syscall/wait_unimp.go
index 810e35d..15b75a6 100644
--- a/src/os/wait_unimp.go
+++ b/src/syscall/wait_unimp.go
@@ -7,15 +7,15 @@
//go:build aix || darwin || (js && wasm) || openbsd || solaris || wasip1
-package os
+package syscall
-// blockUntilWaitable attempts to block until a call to p.Wait will
+// blockUntilWaitable attempts to block until a call to Wait4 will
// succeed immediately, and reports whether it has done so.
-// It does not actually call p.Wait.
+// It does not actually call Wait4.
// This version is used on systems that do not implement waitid,
// or where we have not implemented it yet. Note that this is racy:
// a call to Process.Signal can in an extremely unlikely case send a
// signal to the wrong process, see issue #13987.
-func (p *Process) blockUntilWaitable() (bool, error) {
- return false, nil
+func blockUntilWaitable(pid int) (int, error) {
+ return 0, nil
}
diff --git a/src/syscall/wait_wait6.go b/src/syscall/wait_wait6.go
new file mode 100644
index 0000000..27ddff5
--- /dev/null
+++ b/src/syscall/wait_wait6.go
@@ -0,0 +1,24 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build dragonfly || freebsd || netbsd
+
+package syscall
+
+// blockUntilWaitable attempts to block until a call to Wait4 will
+// succeed immediately, and reports whether it has done so.
+// It does not actually call Wait4.
+func blockUntilWaitable(searchPID int) (int, error) {
+ idType := _P_ALL
+ if searchPID != 0 {
+ idType = _P_PID
+ }
+ for {
+ if pid, err := wait6(idType, searchPID, WEXITED|WNOWAIT); err == ENOSYS {
+ return 0, nil
+ } else if err != EINTR {
+ return pid, err
+ }
+ }
+}
diff --git a/src/syscall/wait_waitid.go b/src/syscall/wait_waitid.go
new file mode 100644
index 0000000..cb30fb4
--- /dev/null
+++ b/src/syscall/wait_waitid.go
@@ -0,0 +1,45 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// We used to used this code for Darwin, but according to issue #19314
+// waitid returns if the process is stopped, even when using WEXITED.
+
+//go:build linux
+
+package syscall
+
+import (
+ "unsafe"
+)
+
+const (
+ _P_ALL = 0
+ _P_PID = 1
+)
+
+// blockUntilWaitable attempts to block until a call to Wait4 will
+// succeed immediately, and reports whether it has done so.
+// It does not actually call Wait4.
+func blockUntilWaitable(searchPID int) (int, error) {
+ // The waitid system call expects a pointer to a siginfo_t,
+ // which is 128 bytes on all Linux systems.
+ // On darwin/amd64, it requires 104 bytes.
+ // We don't care about the values it returns.
+ var siginfo [16]uint64
+ psig := &siginfo[0]
+ idType := _P_ALL
+ if searchPID != 0 {
+ idType = _P_PID
+ }
+ for {
+ if pid, _, err := Syscall6(SYS_WAITID, uintptr(idType), uintptr(searchPID), uintptr(unsafe.Pointer(psig)), WEXITED|WNOWAIT, 0, 0); err == ENOSYS {
+ // waitid has been available since Linux 2.6.9, but
+ // reportedly is not available in Ubuntu on Windows.
+ // See issue 16610.
+ return 0, nil
+ } else if err != EINTR {
+ return int(pid), err
+ }
+ }
+}
To view, visit change 508635. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Jul 10, 2023, 8:51:27 PM7/10/23
to Gerrit Bot, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Ian Lance Taylor, Tobias Klauser, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick, Tobias Klauser.
3 comments:
Patchset:
This seems like a lot of code to handle what sounds like an extremely rare case. Does this ever happen except for people writing an init process? Can people writing an init process simply avoid the os and os/exec packages and just call syscall directly?
File src/syscall/exec_unix.go:
Patch Set #1, Line 496:
type ProcessHandle interface {This is new user-visible API, which requires a proposal. See https://go.dev/s/proposal.
Patch Set #1, Line 536:
func StartProcess(argv0 string, argv []string, attr *ProcAttr) (int, ProcessHandle, error) {This changes the second result from uintptr to ProcessHandle. We can't do that, as it breaks the Go 1 compatibility guarantee (https://go.dev/doc/go1compat).
To view, visit change 508635. To unsubscribe, or for help writing mail filters, visit settings.
Ed Schouten (Gerrit)
Jul 11, 2023, 2:03:02 AM7/11/23
to Gerrit Bot, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Ian Lance Taylor, Tobias Klauser, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor, Tobias Klauser.
3 comments:
Patchset:
This seems like a lot of code to handle what sounds like an extremely rare case. […]
Any Go binary that is the entry point of a Docker container is running as PID 1, so I wouldn't say it's rare in particular.
People could in theory avoid the os and os/exec packages, but that isn't very appealing. There's quite a lot of useful stuff in those packages that you'd have to replicate. For example, os/exec allows terminating a process based on context expiration. This is something that you'd have to hand-roll then.
File src/syscall/exec_unix.go:
Patch Set #1, Line 496:
type ProcessHandle interface {This is new user-visible API, which requires a proposal. See https://go.dev/s/proposal.
Gotcha! I can write a proposal. Just let me know what your preference is on the name for the StartProcess() replacement and I'll make sure to incorporate that.
Patch Set #1, Line 536:
func StartProcess(argv0 string, argv []string, attr *ProcAttr) (int, ProcessHandle, error) {
This changes the second result from uintptr to ProcessHandle. […]
What would your thoughts on adding a new function named StartProcessWithHandle()?
To view, visit change 508635. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Jul 11, 2023, 9:57:07 AM7/11/23
to Gerrit Bot, goph...@pubsubhelper.golang.org, Ed Schouten, Brad Fitzpatrick, Ian Lance Taylor, Tobias Klauser, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick, Ed Schouten, Tobias Klauser.
1 comment:
File src/syscall/exec_unix.go:
Patch Set #1, Line 496:
type ProcessHandle interface {
Gotcha! I can write a proposal. […]
I haven't fully read read the CL, but the description suggests that it does more than is required to run as PID 1. It seems to me that in general a PID 1 Go program shouldn't need to care about child processes that are reparented. The os/exec code should always wait for a known process, not for an arbitrary process. It's not clear to me that we need to make every Go program support reaping zombies just because it is PID 1. For example, I assume that C programs do not routinely do that.
To view, visit change 508635. To unsubscribe, or for help writing mail filters, visit settings.
Ed Schouten (Gerrit)
Jul 11, 2023, 1:01:31 PM7/11/23
to Gerrit Bot, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Ian Lance Taylor, Tobias Klauser, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor, Tobias Klauser.
1 comment:
File src/syscall/exec_unix.go:
Patch Set #1, Line 496:
type ProcessHandle interface {
It seems to me that in general a PID 1 Go program shouldn't need to care about child processes that are reparented.
Apart from contrived use cases where processes create new sessions, daemonize, etc., reparenting may already happen if any direct child of PID 1 terminates (e.g., crashes) before grandchildren do. This is not an uncommon use case if you ask me.
I didn't write this patch just for the sake of it. This issue is a real problem for me for Buildbarn, a distributed build cluster for Bazel (https://github.com/buildbarn). There I see that if I need to kill build actions due to them exceeding their timeout, we often leave processes behind.
I have strongly considered completely banning the use of os/exec in the codebase, but it simply leads to too much code duplication. If there was a way to use os/exec, but swap out the bottom half that does the waiting, I would have already done so.
It's not clear to me that we need to make every Go program support reaping zombies just because it is PID 1. For example, I assume that C programs do not routinely do that.
That's correct. Most C programs also don't do it properly. But a lot of things have changed since the bulk of software built in C was written. For example, Docker became a thing and Google released a tool named Kubernetes that allows you to run Docker containers at scale.
Coming back to the topic of how common it is to run Go binaries as PID 1, I just noticed that if I literally follow the instructions on https://hub.docker.com/_/golang, I end up with exactly that:
$ cat Dockerfile
FROM golang:1.20
WORKDIR /usr/src/app
COPY go.mod ./
RUN go mod download && go mod verify
COPY . .
RUN go build -v -o /usr/local/bin/app ./...
CMD ["app"]
$ cat main.go
package main
import (
"fmt"
"os"
)
func main() {
fmt.Println(os.Getpid())
}
$ docker build -t my-golang-app .
$ docker run -it --rm --name my-running-app my-golang-app
1
To view, visit change 508635. To unsubscribe, or for help writing mail filters, visit settings.
Gopher Robot (Gerrit)
Feb 15, 2025, 6:30:11 AMFeb 15
to Gerrit Bot, goph...@pubsubhelper.golang.org, Ed Schouten, Brad Fitzpatrick, Ian Lance Taylor, Tobias Klauser, golang-co...@googlegroups.com
Gopher Robot abandoned this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages