slices: Clone keeps source array alive when source slice is zero length

[diegommm]

Go version

go version go1.22.5 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/gopher/.cache/go-build'
GOENV='/home/gopher/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/gopher/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/gopher/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/snap/go/10660'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/snap/go/10660/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3287061668=/tmp/go-build -gno-record-gcc-switches'

What did you do?

See this playground.

package main

import (
	"fmt"
	"runtime"
	"slices"
)

func main() {
	fmt.Println("running for standard library")
	cloneWithFunc(slices.Clone[[]*int, *int])
	fmt.Println()
	fmt.Println("running for local func")
	cloneWithFunc(clone[[]*int, *int])
}

func cloneWithFunc(f func([]*int) []*int) {
	x := []*int{newInt(), newInt(), newInt()}
	x = f(x[:0])
	forceGC()
	fmt.Println("did the finalizer for this slice items run before this printed line?", x)
	forceGC()
}

func clone[S ~[]E, E any](s S) S {
	return append(S(nil), s...)
}

func forceGC() {
	runtime.GC()
	runtime.GC()
}

var count int

func newInt() *int {
	count++
	id := count
	x := new(int)
	runtime.SetFinalizer(x, func(*int) {
		fmt.Println("finalizer run for item:", id)
	})
	return x
}

What did you see happen?

running for standard library
did the finalizer for this slice items run before this printed line? []
finalizer run for item: 3
finalizer run for item: 2
finalizer run for item: 1

running for local func
finalizer run for item: 5
finalizer run for item: 4
finalizer run for item: 6
did the finalizer for this slice items run before this printed line? []

What did you expect to see?

running for standard library
finalizer run for item: 3
finalizer run for item: 2
finalizer run for item: 1
did the finalizer for this slice items run before this printed line? []

running for local func
finalizer run for item: 5
finalizer run for item: 4
finalizer run for item: 6
did the finalizer for this slice items run before this printed line? []

[diegommm]

Open to contribute with a CL in case this can be confirmed.

[gabyhelp]

Related Issues and Documentation

• cmd/compile: suboptimal cloning/optimization in slices.Clone #53643
• cmd/compile: a slice captured by a closure might leak #48254 (closed)
• cmd/compile: stringtoslicebytetmp optimization on unescaped slice #38501 (closed)
• x/exp/slices: Clone returns nil for an empty non-nil slice #49660 (closed)
• cmd/compile: unsafe.{SliceData,StringData} possible memory corruption in escape analysis #57823 (closed)
• Garbage Collection with References Leak #51138 (closed)

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

[Jorropo]

See this modified version: https://go.dev/play/p/uSY4HsNVTjQ

running for standard library
did the finalizer for this slice items run before this printed line? {0xc000010018 0 0}
finalizer run for item: 3
finalizer run for item: 2
finalizer run for item: 1

running for local func
finalizer run for item: 4
finalizer run for item: 6
finalizer run for item: 5
did the finalizer for this slice items run before this printed line? {<nil> 0 0}

As well as:

go/src/slices/slices.go

Lines 348 to 349 in 90c6558

	// The s[:0:0] preserves nil in case it matters.
	return append(s[:0:0], s...)

the behaviors are different.

I'll submit a fix thx for the report.

[diegommm]

Jorropo changed the title slices: make Clone allow gc of non-copied items slices: Clone keeps source array alive when source slice is zero length

@Jorropo this also happens when the source slice is non-zero length. In cloneWithFunc, try changing x = f(x[:0]) for x = f(x[:1]). Starting from your version, I get:

running for standard library
finalizer run for item: 3
finalizer run for item: 2
did the finalizer for this slice items run before this printed line? {0xc00004a028 1 1}
finalizer run for item: 1

running for local func
finalizer run for item: 6
finalizer run for item: 5
did the finalizer for this slice items run before this printed line? {0xc00004a000 1 1}
finalizer run for item: 4

[Jorropo]

@diegommm the arrays don't overlap when using x[:1] https://go.dev/play/p/GpymLHZOQFL
Finalizers are janky (such as not being synchronous and reviving references)
I don't think the output above can be interpreted because both the std bugged version and your fixed version give the same output.

[gopherbot]

Change https://go.dev/cl/598875 mentions this issue: slices: prevent Clone keeping alive the array when cloning empty slices

[ianlancetaylor]

I'm not convinced that this is worth fixing. Finalizers don't make any promises. Calling slices.Clone on an empty slice is an unusual action. This seems more like something to cover in a blog post somewhere.

[tdakkota]

@ianlancetaylor

I would expect that slices.Clone always returns slice pointing to a new location.
People might use it in a same way as strings.Clone: to make a copy that does not point
to a larger slice.

[diegommm]

@ianlancetaylor Could you clarify what do you mean by "worth fixing"? Do you see any downside of applying this fix?

[ianlancetaylor]

On the CL @randall77 gave a good rationale for the change, and I'm OK with it.