os: revert the use of AddCleanup to close files and roots

This reverts commit fdaac84480.

Updates #70907
Updates #74574
Updates #74642

Reason for revert: Issue #74574

Change-Id: I7b55b85736e4210d9b6f3fd7a24050ac7bdefef9
Reviewed-on: https://go-review.googlesource.com/c/go/+/688435
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This commit is contained in:
Carlos Amedee 2025-07-16 12:05:48 -07:00
parent 34b70684ba
commit 0451816430
7 changed files with 22 additions and 32 deletions

View file

@ -707,9 +707,9 @@ func (f *File) SyscallConn() (syscall.RawConn, error) {
// Fd returns the system file descriptor or handle referencing the open file.
// If f is closed, the descriptor becomes invalid.
// If f is garbage collected, a cleanup may close the descriptor,
// making it invalid; see [runtime.AddCleanup] for more information on when
// a cleanup might be run.
// If f is garbage collected, a finalizer may close the descriptor,
// making it invalid; see [runtime.SetFinalizer] for more information on when
// a finalizer might be run.
//
// Do not close the returned descriptor; that could cause a later
// close of f to close an unrelated descriptor.

View file

@ -23,7 +23,7 @@ func fixLongPath(path string) string {
// file is the real representation of *File.
// The extra level of indirection ensures that no clients of os
// can overwrite this data, which could cause the cleanup
// can overwrite this data, which could cause the finalizer
// to close the wrong file descriptor.
type file struct {
fdmu poll.FDMutex
@ -31,7 +31,6 @@ type file struct {
name string
dirinfo atomic.Pointer[dirInfo] // nil unless directory being read
appendMode bool // whether file is opened for appending
cleanup runtime.Cleanup // cleanup closes the file when no longer referenced
}
// fd is the Plan 9 implementation of Fd.
@ -49,7 +48,7 @@ func newFileFromNewFile(fd uintptr, name string) *File {
return nil
}
f := &File{&file{sysfd: fdi, name: name}}
f.cleanup = runtime.AddCleanup(f, func(f *file) { f.close() }, f.file)
runtime.SetFinalizer(f.file, (*file).close)
return f
}
@ -160,9 +159,8 @@ func (file *file) close() error {
err := file.decref()
// There is no need for a cleanup at this point. File must be alive at the point
// where cleanup.stop is called.
file.cleanup.Stop()
// no need for a finalizer anymore
runtime.SetFinalizer(file, nil)
return err
}

View file

@ -54,7 +54,7 @@ func rename(oldname, newname string) error {
// file is the real representation of *File.
// The extra level of indirection ensures that no clients of os
// can overwrite this data, which could cause the cleanup
// can overwrite this data, which could cause the finalizer
// to close the wrong file descriptor.
type file struct {
pfd poll.FD
@ -63,7 +63,6 @@ type file struct {
nonblock bool // whether we set nonblocking mode
stdoutOrErr bool // whether this is stdout or stderr
appendMode bool // whether file is opened for appending
cleanup runtime.Cleanup // cleanup closes the file when no longer referenced
}
// fd is the Unix implementation of Fd.
@ -222,8 +221,7 @@ func newFile(fd int, name string, kind newFileKind, nonBlocking bool) *File {
}
}
// Close the file when the File is not live.
f.cleanup = runtime.AddCleanup(f, func(f *file) { f.close() }, f.file)
runtime.SetFinalizer(f.file, (*file).close)
return f
}
@ -320,9 +318,8 @@ func (file *file) close() error {
err = &PathError{Op: "close", Path: file.name, Err: e}
}
// There is no need for a cleanup at this point. File must be alive at the point
// where cleanup.stop is called.
file.cleanup.Stop()
// no need for a finalizer anymore
runtime.SetFinalizer(file, nil)
return err
}

View file

@ -22,14 +22,13 @@ const _UTIME_OMIT = -1
// file is the real representation of *File.
// The extra level of indirection ensures that no clients of os
// can overwrite this data, which could cause the cleanup
// can overwrite this data, which could cause the finalizer
// to close the wrong file descriptor.
type file struct {
pfd poll.FD
name string
dirinfo atomic.Pointer[dirInfo] // nil unless directory being read
appendMode bool // whether file is opened for appending
cleanup runtime.Cleanup // cleanup closes the file when no longer referenced
}
// fd is the Windows implementation of Fd.
@ -69,7 +68,7 @@ func newFile(h syscall.Handle, name string, kind string, nonBlocking bool) *File
},
name: name,
}}
f.cleanup = runtime.AddCleanup(f, func(f *file) { f.close() }, f.file)
runtime.SetFinalizer(f.file, (*file).close)
// Ignore initialization errors.
// Assume any problems will show up in later I/O.
@ -144,9 +143,8 @@ func (file *file) close() error {
err = &PathError{Op: "close", Path: file.name, Err: e}
}
// There is no need for a cleanup at this point. File must be alive at the point
// where cleanup.stop is called.
file.cleanup.Stop()
// no need for a finalizer anymore
runtime.SetFinalizer(file, nil)
return err
}

View file

@ -22,11 +22,10 @@ type root struct {
// refs is incremented while an operation is using fd.
// closed is set when Close is called.
// fd is closed when closed is true and refs is 0.
mu sync.Mutex
fd sysfdType
refs int // number of active operations
closed bool // set when closed
cleanup runtime.Cleanup // cleanup closes the file when no longer referenced
mu sync.Mutex
fd sysfdType
refs int // number of active operations
closed bool // set when closed
}
func (r *root) Close() error {
@ -36,9 +35,7 @@ func (r *root) Close() error {
syscall.Close(r.fd)
}
r.closed = true
// There is no need for a cleanup at this point. Root must be alive at the point
// where cleanup.stop is called.
r.cleanup.Stop()
runtime.SetFinalizer(r, nil) // no need for a finalizer any more
return nil
}

View file

@ -56,7 +56,7 @@ func newRoot(fd int, name string) (*Root, error) {
fd: fd,
name: name,
}}
r.root.cleanup = runtime.AddCleanup(r, func(f *root) { f.Close() }, r.root)
runtime.SetFinalizer(r.root, (*root).Close)
return r, nil
}

View file

@ -113,7 +113,7 @@ func newRoot(fd syscall.Handle, name string) (*Root, error) {
fd: fd,
name: name,
}}
r.root.cleanup = runtime.AddCleanup(r, func(f *root) { f.Close() }, r.root)
runtime.SetFinalizer(r.root, (*root).Close)
return r, nil
}