Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sync: sync.Mutex.* escape m *Mutex argument #57910

Open
Jorropo opened this issue Jan 18, 2023 · 2 comments
Open

sync: sync.Mutex.* escape m *Mutex argument #57910

Jorropo opened this issue Jan 18, 2023 · 2 comments
Assignees
@randall77
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Backlog

Comments

@Jorropo
Copy link
Member

Jorropo commented Jan 18, 2023

What version of Go are you using (go version)?

$ go version
go version go1.19.5 linux/amd64

Does this issue reproduce with the latest release?

Yes as well as current master (2e792a8).

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/hugo/.cache/go-build"
GOENV="/home/hugo/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/hugo/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/hugo/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/home/hugo/Documents/Scripts/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/hugo/Documents/Scripts/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.5"
GCCGO="gccgo"
GOAMD64="v3"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3304808272=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I have some type that do lazy initialization of some state, for that I use sync.Once, however that force my object to leak and prevents stack allocations where I use this object in a non leaky way:
Here is some minimal repro go build -gcflags="-m" a.go:

package a

import (
	"sync"
	"fmt"
)

type A struct {
	o sync.Once
	v int
}

func (a *A) init() {
	a.o.Do(func() {
		a.v = 42
	})
}

func (a *A) Print() {
	a.init()
	fmt.Println(a.v)
}

func main() {
	var a A
	a.Print()
}

What did you expect to see?

# command-line-arguments
./a.go:14:8: inlining call to sync.(*Once).Do
./a.go:14:9: can inline (*A).init.func1
./a.go:21:13: inlining call to fmt.Println
./a.go:24:6: can inline main
./a.go:13:7: leaking param: a
./a.go:14:9: func literal does not escape
./a.go:19:7: leaking param: a
./a.go:21:13: ... argument does not escape
./a.go:21:15: a.v escapes to heap
./a.go:25:6: moved to heap: a

What did you see instead?

# command-line-arguments
./a.go:14:8: inlining call to sync.(*Once).Do
./a.go:14:9: can inline (*A).init.func1
./a.go:21:13: inlining call to fmt.Println
./a.go:24:6: can inline main
./a.go:13:7: a does not escape
./a.go:14:9: func literal does not escape
./a.go:19:7: a does not escape
./a.go:21:13: ... argument does not escape
./a.go:21:15: a.v escapes to heap

Depends on #16241 (note that fixing #16241 wont fully fix the problem here but is required).
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 18, 2023
@Jorropo
Copy link
Member Author

Jorropo commented Jan 18, 2023

Actually it seems smelly, m do infact get leaked in the runtime.

However the runtime will not keep a reference to m if the current goroutine dies and m is not leaked an other way.
AFAIT the only way for m to leak is someone else locks / unlocks the mutex which require m to be leaked an other way (so the other goroutine has access to it in the first place).

So assuming it does not leak is safe ? I think.

@gopherbot
Copy link

Change https://go.dev/cl/462299 mentions this issue: sync: mark some internal runtime go:linkname'd functions noescape

@mknyszek mknyszek added this to the Backlog milestone Jan 18, 2023
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 19, 2023
@Jorropo Jorropo changed the title cmd/compile: incorrectly assume that sync.Mutex.* escape m *Mutex argument sync: sync.Mutex.* escape m *Mutex argument Oct 5, 2023
@mateusz834 mateusz834 mentioned this issue Nov 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@randall77 randall77

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Backlog
Development

No branches or pull requests
6 participants
@mknyszek @bcmills @randall77 @gopherbot @cherrymui @Jorropo