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>

2 files changed, 20 insertions, 0 deletions

diff --git a/src/os/exec.go b/src/os/exec.go
index 6206ae28cd..43b33fe944 100644
--- 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 7a6f4cfad0..5d89c9d39d 100644
--- 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