Skip to content

Commit fdaac84

Browse files
cagedmantisgopherbot
authored andcommitted
os: use AddCleanup to close files
This changes the finalizer mechanism used to close files from runtime.SetFinalizer to runtime.AddCleanup. Updates #70907 Change-Id: I47582b81b0ed69609dd9dac158ec7bb8819c8c77 Reviewed-on: https://go-review.googlesource.com/c/go/+/638555 Reviewed-by: Michael Pratt <[email protected]> Auto-Submit: Carlos Amedee <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent f91ac1b commit fdaac84

File tree

6 files changed

+40
-30
lines changed

6 files changed

+40
-30
lines changed

src/os/file_plan9.go

+9-7
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,22 @@ func fixLongPath(path string) string {
2323

2424
// file is the real representation of *File.
2525
// The extra level of indirection ensures that no clients of os
26-
// can overwrite this data, which could cause the finalizer
26+
// can overwrite this data, which could cause the cleanup
2727
// to close the wrong file descriptor.
2828
type file struct {
2929
fdmu poll.FDMutex
3030
fd int
3131
name string
3232
dirinfo atomic.Pointer[dirInfo] // nil unless directory being read
3333
appendMode bool // whether file is opened for appending
34+
cleanup runtime.Cleanup // cleanup closes the file when no longer referenced
3435
}
3536

3637
// Fd returns the integer Plan 9 file descriptor referencing the open file.
3738
// If f is closed, the file descriptor becomes invalid.
38-
// If f is garbage collected, a finalizer may close the file descriptor,
39-
// making it invalid; see [runtime.SetFinalizer] for more information on when
40-
// a finalizer might be run. On Unix systems this will cause the [File.SetDeadline]
39+
// If f is garbage collected, a cleanup may close the file descriptor,
40+
// making it invalid; see [runtime.AddCleanup] for more information on when
41+
// a cleanup might be run. On Unix systems this will cause the [File.SetDeadline]
4142
// methods to stop working.
4243
//
4344
// As an alternative, see the f.SyscallConn method.
@@ -57,7 +58,7 @@ func NewFile(fd uintptr, name string) *File {
5758
return nil
5859
}
5960
f := &File{&file{fd: fdi, name: name}}
60-
runtime.SetFinalizer(f.file, (*file).close)
61+
f.cleanup = runtime.AddCleanup(f, func(f *file) { f.close() }, f.file)
6162
return f
6263
}
6364

@@ -168,8 +169,9 @@ func (file *file) close() error {
168169

169170
err := file.decref()
170171

171-
// no need for a finalizer anymore
172-
runtime.SetFinalizer(file, nil)
172+
// There is no need for a cleanup at this point. File must be alive at the point
173+
// where cleanup.stop is called.
174+
file.cleanup.Stop()
173175
return err
174176
}
175177

src/os/file_unix.go

+12-9
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func rename(oldname, newname string) error {
5454

5555
// file is the real representation of *File.
5656
// The extra level of indirection ensures that no clients of os
57-
// can overwrite this data, which could cause the finalizer
57+
// can overwrite this data, which could cause the cleanup
5858
// to close the wrong file descriptor.
5959
type file struct {
6060
pfd poll.FD
@@ -63,17 +63,18 @@ type file struct {
6363
nonblock bool // whether we set nonblocking mode
6464
stdoutOrErr bool // whether this is stdout or stderr
6565
appendMode bool // whether file is opened for appending
66+
cleanup runtime.Cleanup // cleanup closes the file when no longer referenced
6667
}
6768

6869
// Fd returns the integer Unix file descriptor referencing the open file.
6970
// If f is closed, the file descriptor becomes invalid.
70-
// If f is garbage collected, a finalizer may close the file descriptor,
71-
// making it invalid; see [runtime.SetFinalizer] for more information on when
72-
// a finalizer might be run. On Unix systems this will cause the [File.SetDeadline]
71+
// If f is garbage collected, a cleanup may close the file descriptor,
72+
// making it invalid; see [runtime.AddCleanup] for more information on when
73+
// a cleanup might be run. On Unix systems this will cause the [File.SetDeadline]
7374
// methods to stop working.
7475
// Because file descriptors can be reused, the returned file descriptor may
75-
// only be closed through the [File.Close] method of f, or by its finalizer during
76-
// garbage collection. Otherwise, during garbage collection the finalizer
76+
// only be closed through the [File.Close] method of f, or by its cleanup during
77+
// garbage collection. Otherwise, during garbage collection the cleanup
7778
// may close an unrelated file descriptor with the same (reused) number.
7879
//
7980
// As an alternative, see the f.SyscallConn method.
@@ -240,7 +241,8 @@ func newFile(fd int, name string, kind newFileKind, nonBlocking bool) *File {
240241
}
241242
}
242243

243-
runtime.SetFinalizer(f.file, (*file).close)
244+
// Close the file when the File is not live.
245+
f.cleanup = runtime.AddCleanup(f, func(f *file) { f.close() }, f.file)
244246
return f
245247
}
246248

@@ -337,8 +339,9 @@ func (file *file) close() error {
337339
err = &PathError{Op: "close", Path: file.name, Err: e}
338340
}
339341

340-
// no need for a finalizer anymore
341-
runtime.SetFinalizer(file, nil)
342+
// There is no need for a cleanup at this point. File must be alive at the point
343+
// where cleanup.stop is called.
344+
file.cleanup.Stop()
342345
return err
343346
}
344347

src/os/file_windows.go

+9-7
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,21 @@ const _UTIME_OMIT = -1
2222

2323
// file is the real representation of *File.
2424
// The extra level of indirection ensures that no clients of os
25-
// can overwrite this data, which could cause the finalizer
25+
// can overwrite this data, which could cause the cleanup
2626
// to close the wrong file descriptor.
2727
type file struct {
2828
pfd poll.FD
2929
name string
3030
dirinfo atomic.Pointer[dirInfo] // nil unless directory being read
3131
appendMode bool // whether file is opened for appending
32+
cleanup runtime.Cleanup // cleanup closes the file when no longer referenced
3233
}
3334

3435
// Fd returns the Windows handle referencing the open file.
3536
// If f is closed, the file descriptor becomes invalid.
36-
// If f is garbage collected, a finalizer may close the file descriptor,
37-
// making it invalid; see [runtime.SetFinalizer] for more information on when
38-
// a finalizer might be run. On Unix systems this will cause the [File.SetDeadline]
37+
// If f is garbage collected, a cleanup may close the file descriptor,
38+
// making it invalid; see [runtime.AddCleanup] for more information on when
39+
// a cleanup might be run. On Unix systems this will cause the [File.SetDeadline]
3940
// methods to stop working.
4041
func (file *File) Fd() uintptr {
4142
if file == nil {
@@ -65,7 +66,7 @@ func newFile(h syscall.Handle, name string, kind string) *File {
6566
},
6667
name: name,
6768
}}
68-
runtime.SetFinalizer(f.file, (*file).close)
69+
f.cleanup = runtime.AddCleanup(f, func(f *file) { f.close() }, f.file)
6970

7071
// Ignore initialization errors.
7172
// Assume any problems will show up in later I/O.
@@ -129,8 +130,9 @@ func (file *file) close() error {
129130
err = &PathError{Op: "close", Path: file.name, Err: e}
130131
}
131132

132-
// no need for a finalizer anymore
133-
runtime.SetFinalizer(file, nil)
133+
// There is no need for a cleanup at this point. File must be alive at the point
134+
// where cleanup.stop is called.
135+
file.cleanup.Stop()
134136
return err
135137
}
136138

src/os/root_openat.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,11 @@ type root struct {
2121
// refs is incremented while an operation is using fd.
2222
// closed is set when Close is called.
2323
// fd is closed when closed is true and refs is 0.
24-
mu sync.Mutex
25-
fd sysfdType
26-
refs int // number of active operations
27-
closed bool // set when closed
24+
mu sync.Mutex
25+
fd sysfdType
26+
refs int // number of active operations
27+
closed bool // set when closed
28+
cleanup runtime.Cleanup // cleanup closes the file when no longer referenced
2829
}
2930

3031
func (r *root) Close() error {
@@ -34,7 +35,9 @@ func (r *root) Close() error {
3435
syscall.Close(r.fd)
3536
}
3637
r.closed = true
37-
runtime.SetFinalizer(r, nil) // no need for a finalizer any more
38+
// There is no need for a cleanup at this point. Root must be alive at the point
39+
// where cleanup.stop is called.
40+
r.cleanup.Stop()
3841
return nil
3942
}
4043

src/os/root_unix.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func newRoot(fd int, name string) (*Root, error) {
5252
fd: fd,
5353
name: name,
5454
}}
55-
runtime.SetFinalizer(r.root, (*root).Close)
55+
r.root.cleanup = runtime.AddCleanup(r, func(f *root) { f.Close() }, r.root)
5656
return r, nil
5757
}
5858

src/os/root_windows.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func newRoot(fd syscall.Handle, name string) (*Root, error) {
109109
fd: fd,
110110
name: name,
111111
}}
112-
runtime.SetFinalizer(r.root, (*root).Close)
112+
r.root.cleanup = runtime.AddCleanup(r, func(f *root) { f.Close() }, r.root)
113113
return r, nil
114114
}
115115

0 commit comments

Comments
 (0)