os: use AddCleanup, not SetFinalizer, for Process

There is no reason to use a cleanup/finalizer for a Process that
doesn't use a handle, because Release doesn't change anything visible
about the process.

For #70907

Change-Id: I3b92809175523ceee2e07d601cc2a8e8b86321e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/638579
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

diff --git a/src/os/exec.go b/src/os/exec.go
index 3f83139da5b2ff1056f46eeb2a91902e8ef23326..212689b0faf0d054f696219c3ab0ff00d402bb8b 100644 (file)
--- a/src/os/exec.go
+++ b/src/os/exec.go
@@ -52,6 +52,9 @@ type Process struct {
        // This is a pointer to a separate memory allocation
        // so that we can use runtime.AddCleanup.
        handle *processHandle
+
+       // cleanup is used to clean up the process handle.
+       cleanup runtime.Cleanup
 }
 
 // processHandle holds an operating system handle to a process.
@@ -111,7 +114,6 @@ func newPIDProcess(pid int) *Process {
        p := &Process{
                Pid: pid,
        }
-       runtime.SetFinalizer(p, (*Process).Release)
        return p
 }
 
@@ -128,7 +130,9 @@ func newHandleProcess(pid int, handle uintptr) *Process {
                Pid:    pid,
                handle: ph,
        }
-       runtime.SetFinalizer(p, (*Process).Release)
+
+       p.cleanup = runtime.AddCleanup(p, (*processHandle).release, ph)
+
        return p
 }
 
@@ -137,7 +141,6 @@ func newDoneProcess(pid int) *Process {
                Pid: pid,
        }
        p.state.Store(uint32(statusDone)) // No persistent reference, as there is no handle.
-       runtime.SetFinalizer(p, (*Process).Release)
        return p
 }
 
@@ -197,7 +200,12 @@ func (p *Process) handlePersistentRelease(reason processStatus) processStatus {
                if !p.state.CompareAndSwap(state, uint32(reason)) {
                        continue
                }
+
+               // No need for more cleanup.
+               p.cleanup.Stop()
+
                p.handle.release()
+
                return status
        }
 }

diff --git a/src/os/exec_plan9.go b/src/os/exec_plan9.go
index bc7a9cdcbc5f743e8b768b28d45f730dabeb180a..6e32a1ae17db359d6bcf8a6ada5f6b6f54dccb79 100644 (file)
--- a/src/os/exec_plan9.go
+++ b/src/os/exec_plan9.go
@@ -6,7 +6,6 @@ package os
 
 import (
        "internal/itoa"
-       "runtime"
        "syscall"
        "time"
 )
@@ -95,8 +94,6 @@ func (p *Process) release() error {
        // Just mark the PID unusable.
        p.pidDeactivate(statusReleased)
 
-       // no need for a finalizer anymore
-       runtime.SetFinalizer(p, nil)
        return nil
 }
 

diff --git a/src/os/exec_unix.go b/src/os/exec_unix.go
index e58801b184cd2f01231c7a65427457716c3b8d58..3b2552eb510d0bfde4a209232f7ba7fd616b889f 100644 (file)
--- a/src/os/exec_unix.go
+++ b/src/os/exec_unix.go
@@ -8,7 +8,6 @@ package os
 
 import (
        "errors"
-       "runtime"
        "syscall"
        "time"
 )
@@ -136,8 +135,7 @@ func (p *Process) release() error {
                // Just mark the PID unusable.
                p.pidDeactivate(statusReleased)
        }
-       // no need for a finalizer anymore
-       runtime.SetFinalizer(p, nil)
+
        return nil
 }
 

diff --git a/src/os/exec_windows.go b/src/os/exec_windows.go
index 969eeb7c217db2c3ee13e325f520e3ae750b54de..0eebe76da32e40bcbf64ccf7c41d8c4a4c80d03b 100644 (file)
--- a/src/os/exec_windows.go
+++ b/src/os/exec_windows.go
@@ -83,8 +83,6 @@ func (p *Process) release() error {
                return syscall.EINVAL
        }
 
-       // no need for a finalizer anymore
-       runtime.SetFinalizer(p, nil)
        return nil
 }