]> Cypherpunks repositories - gostls13.git/commitdiff
os: make FindProcess use pidfd on Linux
authorKir Kolyshkin <kolyshkin@gmail.com>
Thu, 16 Nov 2023 09:42:39 +0000 (01:42 -0800)
committerMichael Pratt <mpratt@google.com>
Tue, 21 May 2024 15:09:50 +0000 (15:09 +0000)
This is a continuation of CL 570036.

Amend FindProcess to use pidfdFind, and make it return a special
Process with Pid of pidDone (-2) if the process is not found.

Amend Wait and Signal to return ErrProcessDone if pid == pidDone.

The alternative to the above would be to make FindProcess return
ErrProcessDone, but this is unexpected and incompatible API change,
as discussed in #65866 and #51246.

For #62654.

Rework of CL 542699 (which got reverted in CL 566476).

Change-Id: Ifb4cd3ad1433152fd72ee685d0b85d20377f8723
Reviewed-on: https://go-review.googlesource.com/c/go/+/570681
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Kirill Kolyshkin <kolyshkin@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
src/os/exec_unix.go
src/os/exec_unix_test.go
src/os/export_linux_test.go
src/os/pidfd_linux.go
src/os/pidfd_linux_test.go
src/os/pidfd_other.go

index 9f5810ff760b98f83e2e2d165f09bc98aefb3c7c..5ac89d17aaf5c39da7f16ac41515e6bd118d5560 100644 (file)
@@ -13,8 +13,19 @@ import (
        "time"
 )
 
+const (
+       // Special values for Process.Pid.
+       pidUnset    = 0
+       pidReleased = -1
+       pidDone     = -2
+)
+
 func (p *Process) wait() (ps *ProcessState, err error) {
-       if p.Pid == -1 {
+       switch p.Pid {
+       case pidDone:
+               return nil, ErrProcessDone
+       case pidReleased:
+               // Process already released.
                return nil, syscall.EINVAL
        }
        // Wait on pidfd if possible; fallback to using pid on ENOSYS.
@@ -67,10 +78,12 @@ func (p *Process) wait() (ps *ProcessState, err error) {
 }
 
 func (p *Process) signal(sig Signal) error {
-       if p.Pid == -1 {
+       switch p.Pid {
+       case pidDone:
+               return ErrProcessDone
+       case pidReleased:
                return errors.New("os: process already released")
-       }
-       if p.Pid == 0 {
+       case pidUnset:
                return errors.New("os: process not initialized")
        }
        s, ok := sig.(syscall.Signal)
@@ -98,15 +111,23 @@ func convertESRCH(err error) error {
 
 func (p *Process) release() error {
        p.pidfdRelease()
-       p.Pid = -1
+       p.Pid = pidReleased
        // no need for a finalizer anymore
        runtime.SetFinalizer(p, nil)
        return nil
 }
 
 func findProcess(pid int) (p *Process, err error) {
-       // NOOP for unix.
-       return newProcess(pid, unsetHandle), nil
+       h, err := pidfdFind(pid)
+       if err == ErrProcessDone {
+               // Can't return an error here since users are not expecting it.
+               // Instead, return a process with Pid=pidDone and let a
+               // subsequent Signal or Wait call catch that.
+               return newProcess(pidDone, unsetHandle), nil
+       }
+       // Ignore all other errors from pidfdFind, as the callers
+       // do not expect them, and we can use pid anyway.
+       return newProcess(pid, h), nil
 }
 
 func (p *ProcessState) userTime() time.Duration {
index 88e1b63a99e14362bb2320dacbc9826ed3d70a9f..69bcdbdad1825e2f66a57a18b1d188bd6e24b72b 100644 (file)
@@ -37,7 +37,10 @@ func TestUNIXProcessAlive(t *testing.T) {
        }
        defer p.Kill()
 
-       proc, _ := FindProcess(p.Pid)
+       proc, err := FindProcess(p.Pid)
+       if err != nil {
+               t.Errorf("OS reported error for running process: %v", err)
+       }
        err = proc.Signal(syscall.Signal(0))
        if err != nil {
                t.Errorf("OS reported error for running process: %v", err)
index 55f83c834f252d8e550edaafc9a265cee4981833..1395f520373e5777ee7966ea5260b64ab7ace4c2 100644 (file)
@@ -9,4 +9,5 @@ var (
        PollSpliceFile      = &pollSplice
        GetPollFDAndNetwork = getPollFDAndNetwork
        CheckPidfdOnce      = checkPidfdOnce
+       PidDone             = pidDone
 )
index 3701be5140506a820e54a6fcc8886ae2f0e604ba..5a830dadb252e537c7dbfd08450d04b12a07ed7c 100644 (file)
@@ -47,6 +47,18 @@ func getPidfd(sysAttr *syscall.SysProcAttr) uintptr {
        return uintptr(*sysAttr.PidFD)
 }
 
+func pidfdFind(pid int) (uintptr, error) {
+       if !pidfdWorks() {
+               return unsetHandle, syscall.ENOSYS
+       }
+
+       h, err := unix.PidFDOpen(pid, 0)
+       if err == nil {
+               return h, nil
+       }
+       return unsetHandle, convertESRCH(err)
+}
+
 func (p *Process) pidfdRelease() {
        // Release pidfd unconditionally.
        handle := p.handle.Swap(unsetHandle)
index d003e8cb1bd5c23d74967e21b1d3c2e6e2d5db0c..2db127b6046eccafc86ecd08be679748cf03ae97 100644 (file)
@@ -5,16 +5,53 @@
 package os_test
 
 import (
+       "internal/testenv"
        "os"
        "testing"
 )
 
-func TestCheckPidfd(t *testing.T) {
-       // This doesn't test anything, but merely allows to check that pidfd
-       // is working (and thus being tested) in CI on some platforms.
+func TestFindProcessViaPidfd(t *testing.T) {
+       testenv.MustHaveGoBuild(t)
+       t.Parallel()
+
        if err := os.CheckPidfdOnce(); err != nil {
-               t.Log("checkPidfd:", err)
-       } else {
-               t.Log("pidfd syscalls work")
+               // Non-pidfd code paths tested in exec_unix_test.go.
+               t.Skipf("skipping: pidfd not available: %v", err)
+       }
+
+       p, err := os.StartProcess(testenv.GoToolPath(t), []string{"go"}, &os.ProcAttr{})
+       if err != nil {
+               t.Fatalf("starting test process: %v", err)
+       }
+       p.Wait()
+
+       // Use pid of a non-existing process.
+       proc, err := os.FindProcess(p.Pid)
+       // FindProcess should never return errors on Unix.
+       if err != nil {
+               t.Fatalf("FindProcess: got error %v, want <nil>", err)
+       }
+       // FindProcess should never return nil Process.
+       if proc == nil {
+               t.Fatal("FindProcess: got nil, want non-nil")
+       }
+       if proc.Pid != os.PidDone {
+               t.Fatalf("got pid: %v, want %d", proc.Pid, os.PidDone)
+       }
+
+       // Check that all Process' public methods work as expected with
+       // "done" Process.
+       if err := proc.Kill(); err != os.ErrProcessDone {
+               t.Errorf("Kill: got %v, want %v", err, os.ErrProcessDone)
+       }
+       if err := proc.Signal(os.Kill); err != os.ErrProcessDone {
+               t.Errorf("Signal: got %v, want %v", err, os.ErrProcessDone)
+       }
+       if _, err := proc.Wait(); err != os.ErrProcessDone {
+               t.Errorf("Wait: got %v, want %v", err, os.ErrProcessDone)
+       }
+       // Release never returns errors on Unix.
+       if err := proc.Release(); err != nil {
+               t.Fatalf("Release: got %v, want <nil>", err)
        }
 }
index 1918acbec5b7658e55224cdeebb2dfb01cbb2e44..bb38c724042bc27c545dce71b241a8ec5e6e87db 100644 (file)
@@ -16,6 +16,10 @@ func getPidfd(_ *syscall.SysProcAttr) uintptr {
        return unsetHandle
 }
 
+func pidfdFind(_ int) (uintptr, error) {
+       return unsetHandle, syscall.ENOSYS
+}
+
 func (p *Process) pidfdRelease() {}
 
 func (_ *Process) pidfdWait() (*ProcessState, error) {