os: possible to remove or change internal finalizer on a Root

[mknyszek]

The implementation of os.Root sets its finalizer on the Root itself (it targets the internal root, but that struct is just embedded in Root) meaning that callers can call SetFinalizer on a Root to mutate the finalizer. Other APIs in std, like File, explicitly make this impossible by setting the finalizer on an internal object (a File contains a pointer to a file, which actually has the finalizer).

The main issue with this behavior is that it exposes an internal implementation detail, and may prevent switching from finalizers to cleanups in the future (since the cleanup is not removable unless you have the handle to it). The fix is straightforward: the internal root should be stored by reference in Root.

CC @neild

[neild]

Agreed that this is a bug.

I have no opinion on whether we should get it into 1.24, or just backport into 1.24.1. It's pretty obscure.

[gopherbot]

Change https://go.dev/cl/647876 mentions this issue: os: hide SetFinalizer from users of Root

[dmitshur]

Since this release-blocker is in the Go 1.24 milestone, re-opening to track cherry-picking to release-branch.go1.24.

[gopherbot]

Change https://go.dev/cl/648135 mentions this issue: [release-branch.go1.24] os: hide SetFinalizer from users of Root

[dmitshur]

Closed by merging CL 648135 (commit 6d399e9) to release-branch.go1.24.

[andig]

It feels like a newbie question, but why does making root a pointer hide SetFinalizer? root ist not embedded? Because SetFinalizer works on the address (i.e. allocation reference) of the object? Please ignore me if too OT, will move question to golang-nuts.

[neild]

A finalizer is associated with an allocation, and there's at most one finalizer per allocation. CL 648135 changes a Root into two allocations: type Root struct { *root }, and sets the finalizer into inner one (Root.root). This means that the outer *Root object doesn't have a finalizer associated with it, and if a user creates one they won't interfere with the inner *root's finalizer.

(runtime.AddCleanup, coming in Go 1.24, (see #67535) avoids this and many other problems with finalizers. We aren't using AddCleanup on Root yet because Root and AddCleanup were developed in parallel and I didn't think of it. There's work in progress to switch all internal usage of finalizers to AddCleanup in 1.25, see #70907.)