runtime: AddCleanup is not robust to arg values pointing to struct embedded data

[matttproud]

Go version

go version go1.25-20250216-RC00 cl/727547642 +d524e1eccd X:fieldtrack,boringcrypto linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/usr/local/google/home/mtp/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/usr/local/google/home/mtp/.config/go/env'
GOEXE=''
GOEXPERIMENT='fieldtrack,boringcrypto'
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3976460735=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/dev/null'
GOMODCACHE='/usr/local/google/home/mtp/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/usr/local/google/home/mtp/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/google-golang'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/usr/local/google/home/mtp/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/google-golang/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.25-20250216-RC00 cl/727547642 +d524e1eccd X:fieldtrack,boringcrypto'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I noticed that runtime.AddCleanup does not robustly handle a case like this:

type NeedsCleanup struct { ... }
func (c *NeedsCleanup) Close() { ... }

type Embeds struct { NeedsCleanup }

func New() *Embeds {
  var e Embeds
  runtime.AddCleanup(&e, (*NeedsCleanup).Close, &e.NeedsCleanup)
  ...
  return &e
}

What did you see happen?

The runtime panics:

panic: runtime.AddCleanup: ptr is equal to arg, cleanup will never run

What did you expect to see?

I would expect that this snippet of code to be more robust to the case of struct embedding (if that is at all possible), or the documentation for runtime.AddFunc also mention that naive struct embedding of the data that needs to be cleaned up into an outer struct won't work due to how alignment of memory works.

We found a workaround by amending the definition of the outer type as follows:

type NeedsCleanup struct { ... }
func (c *NeedsCleanup) Close() { ... }

type Embeds struct {
  _ int  // Ensures that NeedsCleanup's address does not align with Embeds for finalization.
  NeedsCleanup
}

func New() *Embeds {
  var e Embeds
  runtime.AddCleanup(&e, (*NeedsCleanup).Close, &e.NeedsCleanup)
  ...
  return &e
}

[gabyhelp]

Related Issues

• runtime: AddCleanup does not panic if arg equals ptr #71316 (closed)
• cmd/compile: unsafe.{SliceData,StringData} possible memory corruption in escape analysis #57823 (closed)
• runtime: go runtime GC panic #45401 (closed)
• runtime: cgoCheckPointer fatal error: can't happen #70016 (closed)
• go build use -asan it panic. #69225 (closed)
• runtime: panic marked free object in span[1.17] #54247 (closed)
• runtime: Invalid garbage collection of structure pointers #36569 (closed)

Related Code Changes

• runtime: fix the equality check in AddCleanup
• runtime: implement AddCleanup

Related Documentation

• Package runtime > type Cleanup ¶ 1.24 > func AddCleanup ¶

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

[mknyszek]

We found a workaround by amending the definition of the outer type as follows:

FTR, the workaround will still result in the cleanup never running. Cleanups apply to contiguous blocks of memory, what we call an "object," not to Go values by the spec. The only fix is to have a pointer indirection:

type NeedsCleanup struct { ... }
func (c *NeedsCleanup) Close() { ... }

type Embeds struct {
  *NeedsCleanup
}

func New() *Embeds {
  var e Embeds
  e.NeedsCleanup = new(NeedsCleanup)
  runtime.AddCleanup(&e, (*NeedsCleanup).Close, e.NeedsCleanup)
  ...
  return &e
}

We don't panic in the workaround case you provided because we cannot easily catch all the cases where a reference from a cleanup back to the object is a problem. I do have a patch for a GODEBUG mode that identifies when cleanups will never run with absolute certainty and crashes the program, but it has a potentially non-trivial cost. (https://go.dev/cl/634599)

[matttproud]

Thank you for clarifying. Would you folks be OK with potentially a few small amendments to the documentation for runtime.AddCleanup? A tracer shot example proposal:

From:

// If ptr is reachable from cleanup or arg, ptr will never be collected and the cleanup will never run.
// As a protection against simple cases of this, AddCleanup panics if arg is equal to ptr.

To:

// If ptr is reachable from cleanup or arg, ptr will never be collected and the cleanup will never run.
// As a protection against simple cases of this, AddCleanup panics if arg is equal to ptr.  ptr still counts
// as being reachable if arg points to something in ptr's memory space, as would be the case with
// struct fields.  A workaround in the struct field case could be to use pointer indirection instead:
//
//  type Client struct {
//    r *resource
//  }
//
//  type resource struct { ... }
//
//  func (r *resource) Close() { ... }
//
//  func New() *Client {
//    c := &Client{r: &resource{...}}
//    runtime.AddCleanup(c, (*resource).Close, c.r)
//    return c
//  }

This is an API that I would rarely use, but I would really benefit from a plain language explanation about how to use it correctly and how not to.

[mknyszek]

Yes, please feel free to send a documentation change! (Or an example too, if you're up for it. :))

[gopherbot]

Change https://go.dev/cl/659975 mentions this issue: runtime: update AddCleanup documentation to note embedded data caveat

[mknyszek]

@iscultas That code is fine. It's passing a copy of slicePipeFields. There would only be an issue if the argument was &p.slicePipeFields.