runtime: allow cleanups to run concurrently

[mknyszek]

Proposal Details

Currently the docs for runtime.AddCleanup state:

A single goroutine runs all cleanup calls for a program, sequentially.

But this was copied from runtime.SetFinalizer because it was convenient. It really deserved more thought.

Since runtime.AddCleanup just debuted in Go 1.24, and thus there probably aren't too many programs actually depending on this behavior, I propose we remove this sentence from the docs and intentionally run cleanups concurrently (on two goroutines to start with) to prevent dependence on this behavior.

We can always change the behavior back to sequential in a backwards-compatible way. We can still recommend that people shunt long-running work to a separate goroutine.

This proposal is partially motivated by #71772, in which sub-optimal usage of unique.Make may result in a rate of insertions into unique's internal map that exceeds the pace of deletions, resulting in unbounded memory growth. The current deletion strategy involves a goroutine that wastefully walks over the whole map. Using runtime.AddCleanup mitigates this problem by directly targeting dead entries in the map. However, this is only a mitigation, and not a full solution. Today, to fully resolve the issue, we will have to shunt work to a global work queue that's specific to the unique package using runtime.AddCleanup, and introduce a new backpressure mechanism in unique.Make. Even so, the cleanup/finalizer goroutine can still theoretically become a bottleneck at high GOMAXPROCS (think 128+). Worse still, anyone else using runtime.AddCleanup to build a canonicalization map or cache has a chance of running into the same problem, and must roll their own solution. This issue has been demonstrated in a small reproducer, where GOMAXPROCS doesn't have to be very high at all, though it hasn't yet been demonstrated in production.

Even if we don't have a concrete plan yet on how exactly to support such uses of runtime.AddCleanup, I think we should at least open the door to alternative cleanup strategies.

[gabyhelp]

Related Issues

• runtime: add AddCleanup #67535 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[aclements]

Because this affects the 1.24 release that just happened, we're planning to fast-track this proposal. It doesn't seem too controversial, and the impact should be minimal because this API was only introduced in 1.24.

[aclements]

@mknyszek , can you write exactly what that paragraph of the AddCleanup documentation would be after this change?

[mknyszek]

Here's a draft diff:

-// A single goroutine runs all cleanup calls for a program, sequentially. If a
-// cleanup function must run for a long time, it should create a new goroutine.
+// Cleanups may run concurrently with one another, unlike finalizers. However, if
+// a cleanup function must run for a long time, it should create a new goroutine
+// to avoid blocking the execution of other cleanups.

[gopherbot]

Change https://go.dev/cl/650697 mentions this issue: runtime: schedule cleanups across multiple goroutines

[gopherbot]

Change https://go.dev/cl/650696 mentions this issue: runtime: document that cleanups can run concurrently with each other

[aclements]

@cherrymui notes that this text allows cleanups to run on a regular user goroutine (e.g., the one calling AddCleanup). It's tempting to allow this because it could be a useful back-pressure mechanism. However, if we were to run a cleanup on a regular user goroutine, it could cause deadlocks; for example, if a goroutine is holding lock L and calls AddCleanup, which then synchronously runs a cleanup that itself tries to acquire lock L. Thus, we should be specific that that the cleanups run concurrently with all user-created goroutines.

I think other than that detail, the proposal committee is good with this change.

[aclements]

Based on the discussion above, this proposal seems like a likely accept.
— aclements for the proposal review group

The proposal is to remove the following sentences from the runtime.AddCleanup documentation:

// A single goroutine runs all cleanup calls for a program, sequentially. If a
// cleanup function must run for a long time, it should create a new goroutine.

and replace them with

// Cleanups run concurrently with any user-created goroutines.
// Cleanups may also run concurrently with one another (unlike finalizers).
// If a cleanup function must run for a long time, it should create a new goroutine
// to avoid blocking the execution of other cleanups.