]> Cypherpunks repositories - gostls13.git/commitdiff
os: revert the use of AddCleanup to close files and roots
authorCarlos Amedee <carlos@golang.org>
Wed, 16 Jul 2025 19:05:48 +0000 (12:05 -0700)
committerCarlos Amedee <carlos@golang.org>
Fri, 18 Jul 2025 21:43:56 +0000 (14:43 -0700)
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
src/os/file.go
src/os/file_plan9.go
src/os/file_unix.go
src/os/file_windows.go
src/os/root_openat.go
src/os/root_unix.go
src/os/root_windows.go

index 9603ac61e6b003f582d9e868fbdd95486939bb36..66269c199e7d95309c72b41f2522232ffbbb6c09 100644 (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.
index 656a3e0bb0eecdd3f73721b1a39cb9cf644db666..17026409eb6021bbea0617b8a10ef320cc56c501 100644 (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
 }
 
index 721f08c911b4552a7a813329a56a709b853f5ec7..2074df70febc2eaad96a938990ccb065b875564d 100644 (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
 }
 
index ee6735fe443137d57f7d32f40be6177bac6cf2df..7e943287109a5f1defaf0ff86707f0f609d0be98 100644 (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
 }
 
index 192c29e319d3946cd4aea41e73efb3eb54bc5cc7..e433bd5093fdb014f42a4fb487ac58019fa0b20a 100644 (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
 }
 
index ed21afffb56755beac188e270a5273d2dfb737a2..4d6fc19a080434a6dbf46ab3c058afc57947be2f 100644 (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
 }
 
index a9186068062483d13d29bead25295566e47f2eae..523ee48d13f8c00fae97e60ac2b1cb1bd465af4b 100644 (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
 }