Skip to content

Commit bdbc5ca

Browse files
ianlancetaylorgopherbot
authored andcommitted
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 <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]> Reviewed-by: Michael Knyszek <[email protected]>
1 parent 8028731 commit bdbc5ca

File tree

4 files changed

+12
-11
lines changed

4 files changed

+12
-11
lines changed

src/os/exec.go

+11-3
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ type Process struct {
5252
// This is a pointer to a separate memory allocation
5353
// so that we can use runtime.AddCleanup.
5454
handle *processHandle
55+
56+
// cleanup is used to clean up the process handle.
57+
cleanup runtime.Cleanup
5558
}
5659

5760
// processHandle holds an operating system handle to a process.
@@ -111,7 +114,6 @@ func newPIDProcess(pid int) *Process {
111114
p := &Process{
112115
Pid: pid,
113116
}
114-
runtime.SetFinalizer(p, (*Process).Release)
115117
return p
116118
}
117119

@@ -128,7 +130,9 @@ func newHandleProcess(pid int, handle uintptr) *Process {
128130
Pid: pid,
129131
handle: ph,
130132
}
131-
runtime.SetFinalizer(p, (*Process).Release)
133+
134+
p.cleanup = runtime.AddCleanup(p, (*processHandle).release, ph)
135+
132136
return p
133137
}
134138

@@ -137,7 +141,6 @@ func newDoneProcess(pid int) *Process {
137141
Pid: pid,
138142
}
139143
p.state.Store(uint32(statusDone)) // No persistent reference, as there is no handle.
140-
runtime.SetFinalizer(p, (*Process).Release)
141144
return p
142145
}
143146

@@ -197,7 +200,12 @@ func (p *Process) handlePersistentRelease(reason processStatus) processStatus {
197200
if !p.state.CompareAndSwap(state, uint32(reason)) {
198201
continue
199202
}
203+
204+
// No need for more cleanup.
205+
p.cleanup.Stop()
206+
200207
p.handle.release()
208+
201209
return status
202210
}
203211
}

src/os/exec_plan9.go

-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package os
66

77
import (
88
"internal/itoa"
9-
"runtime"
109
"syscall"
1110
"time"
1211
)
@@ -95,8 +94,6 @@ func (p *Process) release() error {
9594
// Just mark the PID unusable.
9695
p.pidDeactivate(statusReleased)
9796

98-
// no need for a finalizer anymore
99-
runtime.SetFinalizer(p, nil)
10097
return nil
10198
}
10299

src/os/exec_unix.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package os
88

99
import (
1010
"errors"
11-
"runtime"
1211
"syscall"
1312
"time"
1413
)
@@ -136,8 +135,7 @@ func (p *Process) release() error {
136135
// Just mark the PID unusable.
137136
p.pidDeactivate(statusReleased)
138137
}
139-
// no need for a finalizer anymore
140-
runtime.SetFinalizer(p, nil)
138+
141139
return nil
142140
}
143141

src/os/exec_windows.go

-2
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ func (p *Process) release() error {
8383
return syscall.EINVAL
8484
}
8585

86-
// no need for a finalizer anymore
87-
runtime.SetFinalizer(p, nil)
8886
return nil
8987
}
9088

0 commit comments

Comments
 (0)