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

testing: benchmark ReportMetric panic on concurrent call #54037

Closed
Jack-Kingdom opened this issue Jul 25, 2022 · 4 comments
Closed

testing: benchmark ReportMetric panic on concurrent call #54037

Jack-Kingdom opened this issue Jul 25, 2022 · 4 comments

Comments

@Jack-Kingdom
Copy link

Jack-Kingdom commented Jul 25, 2022

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

$ go version
go version go1.18.4 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE="auto"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jack/Library/Caches/go-build"
GOENV="/Users/jack/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/jack/.go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jack/.go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.18.4/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.18.4/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/zv/mhvsb81s63b7kk08sfhfq8d00000gn/T/go-build1807741596=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

example code: https://go.dev/play/p/ToeyneuJPcT

What did you expect to see?

no panic here

@Jack-Kingdom
Copy link
Author

Jack-Kingdom commented Jul 25, 2022

Copy panic log here

fatal error: concurrent map writes
fatal error: concurrent map writes

goroutine 1200 [running]:
runtime.throw({0x4ae646?, 0x0?})
	/usr/local/go-faketime/src/runtime/panic.go:992 +0x71 fp=0xc0001de740 sp=0xc0001de710 pc=0x431e91
runtime.mapassign_faststr(0x4ab755?, 0x2?, {0x4ab755, 0x2})
	/usr/local/go-faketime/src/runtime/map_faststr.go:212 +0x39c fp=0xc0001de7a8 sp=0xc0001de740 pc=0x4114bc
testing.(*B).ReportMetric(...)
	/usr/local/go-faketime/src/testing/benchmark.go:356
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:7 +0x65 fp=0xc0001de7e0 sp=0xc0001de7a8 pc=0x48e665
runtime.goexit()
	/usr/local/go-faketime/src/runtime/asm_amd64.s:1571 +0x1 fp=0xc0001de7e8 sp=0xc0001de7e0 pc=0x45cdc1
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1 [chan receive]:
testing.(*B).doBench(0xc0000c4000)
	/usr/local/go-faketime/src/testing/benchmark.go:285 +0x7f
testing.(*B).run(0xc0000c4000?)
	/usr/local/go-faketime/src/testing/benchmark.go:279 +0x6e
testing.Benchmark(0x4b3f48)
	/usr/local/go-faketime/src/testing/benchmark.go:824 +0xf5
main.main()
	/tmp/sandbox1758758439/prog.go:18 +0x25

goroutine 59 [runnable]:
main.BenchmarkReportMetrics(0xc0000c4000)
	/tmp/sandbox1758758439/prog.go:12 +0x87
testing.(*B).runN(0xc0000c4000, 0x2710)
	/usr/local/go-faketime/src/testing/benchmark.go:193 +0x102
testing.(*B).launch(0xc0000c4000)
	/usr/local/go-faketime/src/testing/benchmark.go:334 +0x1c5
created by testing.(*B).doBench
	/usr/local/go-faketime/src/testing/benchmark.go:284 +0x6c

goroutine 1393 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1391 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1394 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1277 [running]:
	goroutine running on other thread; stack unavailable
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1392 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1396 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1395 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1389 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1388 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1387 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1386 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1390 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1385 [runnable]:
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:6
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

goroutine 1277 [running]:
runtime.throw({0x4ae646?, 0x0?})
	/usr/local/go-faketime/src/runtime/panic.go:992 +0x71 fp=0xc000186f40 sp=0xc000186f10 pc=0x431e91
runtime.mapassign_faststr(0x49a6e0, 0xc000012030, {0x4ab755, 0x2})
	/usr/local/go-faketime/src/runtime/map_faststr.go:295 +0x38b fp=0xc000186fa8 sp=0xc000186f40 pc=0x4114ab
testing.(*B).ReportMetric(...)
	/usr/local/go-faketime/src/testing/benchmark.go:356
main.BenchmarkReportMetrics.func1()
	/tmp/sandbox1758758439/prog.go:7 +0x65 fp=0xc000186fe0 sp=0xc000186fa8 pc=0x48e665
runtime.goexit()
	/usr/local/go-faketime/src/runtime/asm_amd64.s:1571 +0x1 fp=0xc000186fe8 sp=0xc000186fe0 pc=0x45cdc1
created by main.BenchmarkReportMetrics
	/tmp/sandbox1758758439/prog.go:12 +0x87

Program exited.

@Jack-Kingdom
Copy link
Author

Jack-Kingdom commented Jul 25, 2022

try aquire lock on benchmark extra map write
#54038

@seankhliao
Copy link
Member

seankhliao commented Jul 25, 2022

This looks like an incorrect use of ReportMetric though

@Jack-Kingdom Jack-Kingdom changed the title affected/package: benchmark ReportMetric panic on concurrent call testing: benchmark ReportMetric panic on concurrent call Jul 25, 2022
@bcmills
Copy link
Member

bcmills commented Jul 25, 2022

This looks like an incorrect use of ReportMetric though

I agree, especially given this sentence in its documentation:

ReportMetric overrides any previously reported value for the same unit.

Without a happens-before relationship between the calls to ReportMetric, the notion of “previously reported” is not well defined.

By convention, Go methods that mutate the state of a data-structure (as ReportMetric does) should be considered unsafe for concurrent use unless the documentation explicitly says otherwise.

@bcmills bcmills closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants