From: Carlos Amedee Date: Wed, 16 Jul 2025 19:05:48 +0000 (-0700) Subject: os: revert the use of AddCleanup to close files and roots X-Git-Tag: go1.25rc3~5^2~11 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=0451816430;p=gostls13.git os: revert the use of AddCleanup to close files and roots This reverts commit fdaac84480b02e600660d0ca7c15339138807107. 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 Reviewed-by: Michael Knyszek --- diff --git a/src/os/file.go b/src/os/file.go index 9603ac61e6..66269c199e 100644 --- a/src/os/file.go +++ b/src/os/file.go @@ -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. diff --git a/src/os/file_plan9.go b/src/os/file_plan9.go index 656a3e0bb0..17026409eb 100644 --- a/src/os/file_plan9.go +++ b/src/os/file_plan9.go @@ -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 } diff --git a/src/os/file_unix.go b/src/os/file_unix.go index 721f08c911..2074df70fe 100644 --- a/src/os/file_unix.go +++ b/src/os/file_unix.go @@ -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 } diff --git a/src/os/file_windows.go b/src/os/file_windows.go index ee6735fe44..7e94328710 100644 --- a/src/os/file_windows.go +++ b/src/os/file_windows.go @@ -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 } diff --git a/src/os/root_openat.go b/src/os/root_openat.go index 192c29e319..e433bd5093 100644 --- a/src/os/root_openat.go +++ b/src/os/root_openat.go @@ -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 } diff --git a/src/os/root_unix.go b/src/os/root_unix.go index ed21afffb5..4d6fc19a08 100644 --- a/src/os/root_unix.go +++ b/src/os/root_unix.go @@ -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 } diff --git a/src/os/root_windows.go b/src/os/root_windows.go index a918606806..523ee48d13 100644 --- a/src/os/root_windows.go +++ b/src/os/root_windows.go @@ -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 }