os: improve comments for process support, minor code cleanup

Change-Id: I97ecbc6fc0c73c6d8469144f86a7ad8c2655a658
Reviewed-on: https://go-review.googlesource.com/c/go/+/638581
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>

diff --git a/src/os/exec.go b/src/os/exec.go
index 6206ae28cd3fea54f081447ebf7550cb0036b388..43b33fe94428648cc1cd3802336426f0e91b4e69 100644 (file)
--- a/src/os/exec.go
+++ b/src/os/exec.go
@@ -17,6 +17,7 @@ import (
 // ErrProcessDone indicates a [Process] has finished.
 var ErrProcessDone = errors.New("os: process already finished")
 
+// processStatus describes the status of a [Process].
 type processStatus uint32
 
 const (
@@ -110,6 +111,7 @@ func (ph *processHandle) release() {
        }
 }
 
+// newPIDProcess returns a [Process] for the given PID.
 func newPIDProcess(pid int) *Process {
        p := &Process{
                Pid: pid,
@@ -117,6 +119,7 @@ func newPIDProcess(pid int) *Process {
        return p
 }
 
+// newHandleProcess returns a [Process] with the given PID and handle.
 func newHandleProcess(pid int, handle uintptr) *Process {
        ph := &processHandle{
                handle: handle,
@@ -136,6 +139,9 @@ func newHandleProcess(pid int, handle uintptr) *Process {
        return p
 }
 
+// newDoneProcess returns a [Process] for the given PID
+// that is already marked as done. This is used on Unix systems
+// if the process is known to not exist.
 func newDoneProcess(pid int) *Process {
        p := &Process{
                Pid: pid,
@@ -144,6 +150,8 @@ func newDoneProcess(pid int) *Process {
        return p
 }
 
+// handleTransientAcquire returns the process handle or,
+// if the process is not ready, the current status.
 func (p *Process) handleTransientAcquire() (uintptr, processStatus) {
        if p.handle == nil {
                panic("handleTransientAcquire called in invalid mode")
@@ -169,6 +177,7 @@ func (p *Process) handleTransientAcquire() (uintptr, processStatus) {
        return 0, status
 }
 
+// handleTransientRelease releases a handle returned by handleTransientAcquire.
 func (p *Process) handleTransientRelease() {
        if p.handle == nil {
                panic("handleTransientRelease called in invalid mode")
@@ -176,6 +185,7 @@ func (p *Process) handleTransientRelease() {
        p.handle.release()
 }
 
+// pidStatus returns the current process status.
 func (p *Process) pidStatus() processStatus {
        if p.handle != nil {
                panic("pidStatus called in invalid mode")
@@ -301,6 +311,10 @@ func (p *Process) doRelease(newStatus processStatus) processStatus {
                // created in newHandleProcess.
                if p.handle != nil {
                        // No need for more cleanup.
+                       // We must stop the cleanup before calling release;
+                       // otherwise the cleanup might run concurrently
+                       // with the release, which would cause the reference
+                       // counts to be invalid, causing a panic.
                        p.cleanup.Stop()
 
                        p.handle.release()

diff --git a/src/os/pidfd_linux.go b/src/os/pidfd_linux.go
index 7a6f4cfad0b17729d3d065955302ab88c15defdc..5d89c9d39d60b75609d18b39cfe93618c3c23867 100644 (file)
--- a/src/os/pidfd_linux.go
+++ b/src/os/pidfd_linux.go
@@ -66,6 +66,7 @@ func getPidfd(sysAttr *syscall.SysProcAttr, needDup bool) (uintptr, bool) {
        return uintptr(h), true
 }
 
+// pidfdFind returns the process handle for pid.
 func pidfdFind(pid int) (uintptr, error) {
        if !pidfdWorks() {
                return 0, syscall.ENOSYS
@@ -78,6 +79,8 @@ func pidfdFind(pid int) (uintptr, error) {
        return h, nil
 }
 
+// pidfdWait waits for the process to complete,
+// and updates the process status to done.
 func (p *Process) pidfdWait() (*ProcessState, error) {
        // When pidfd is used, there is no wait/kill race (described in CL 23967)
        // because the PID recycle issue doesn't exist (IOW, pidfd, unlike PID,
@@ -120,6 +123,7 @@ func (p *Process) pidfdWait() (*ProcessState, error) {
        }, nil
 }
 
+// pidfdSendSignal sends a signal to the process.
 func (p *Process) pidfdSendSignal(s syscall.Signal) error {
        handle, status := p.handleTransientAcquire()
        switch status {
@@ -133,10 +137,12 @@ func (p *Process) pidfdSendSignal(s syscall.Signal) error {
        return convertESRCH(unix.PidFDSendSignal(handle, s))
 }
 
+// pidfdWorks returns whether we can use pidfd on this system.
 func pidfdWorks() bool {
        return checkPidfdOnce() == nil
 }
 
+// checkPidfdOnce is used to only check whether pidfd works once.
 var checkPidfdOnce = sync.OnceValue(checkPidfd)
 
 // checkPidfd checks whether all required pidfd-related syscalls work. This