-
Notifications
You must be signed in to change notification settings - Fork 18k
runtime: gcAssistAlloc can call itself recursively #12894
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
Comments
CL https://golang.org/cl/15720 mentions this issue. |
If gcAssistAlloc is unable to steal or perform enough scan work, it calls timeSleep, which allocates. If this allocation requires obtaining a new span, it will in turn attempt to assist GC. Since there's likely still no way to satisfy the assist, it will sleep again, and so on, leading to potentially deep (not infinite, but also not bounded) recursion. Fix this by disallowing assists during the timeSleep. This same problem was fixed on master by 65aa2da. That commit built on several other changes and hence can't be directly cherry-picked. This commit implements the same idea. Fixes #12894. Change-Id: I152977eb1d0a3005c42ff3985d58778f054a86d4 Reviewed-on: https://go-review.googlesource.com/15720 Reviewed-by: Rick Hudson <rlh@golang.org>
Is this fixed now? In both 1.5 branch and tip? |
Yes. It was fixed on master by 65aa2d6 and on the 1.5 branch by c257dfb. |
The fix on the 1.5 branch (c257dfb) isn't quite correct. We were able to verify that it reduces but does not eliminate the issue. I believe the recursion can still happen at the beginning of a GC cycle when gp.gcscanwork is also small or zero. I'm working on a better fix, which we'll try to verify more thoroughly. |
CL https://golang.org/cl/15891 mentions this issue. |
Commit c257dfb attempted to fix recursive allocation in gcAssistAlloc; however, it only reduced it: setting gp.gcalloc to 0 isn't sufficient to disable assists at the beginning of the GC cycle when gp.gcscanwork is also small or zero. Fix this recursion more completely by setting gcalloc to a sentinel value that directly disables assists. Fixes #12894 (again). Change-Id: I9599566222d8f540d0b39806846bfc702e6666e5 Reviewed-on: https://go-review.googlesource.com/15891 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Fixed (for real this time) on release branch by 71a7647. |
This occurs in Go 1.5.1 possibly 1.5.0 as well.
gcAssistAlloc may decide to sleep for 200us. This results in allocating a timer object and eventually ending up in gcAssistAlloc again.
gcAssistAlloc -> time.Sleep -> builtin.new -> mallocgc -> gcAssistAlloc -> ...
sanitized snippet from debug/pprof/heap?debug=1
# 0x123456 time.Sleep+0x37 go/gc/src/runtime/time.go:53
# 0x123456 runtime.gcAssistAlloc+0x23e go/gc/src/runtime/mgcmark.go:295
# 0x123456 runtime.mallocgc+0x535 go/gc/src/runtime/malloc.go:711
# 0x123456 runtime.newobject+0x42 go/gc/src/runtime/malloc.go:760
# 0x123456 time.Sleep+0x37 go/gc/src/runtime/time.go:53
# 0x123456 runtime.gcAssistAlloc+0x23e go/gc/src/runtime/mgcmark.go:295
# 0x123456 runtime.mallocgc+0x535 go/gc/src/runtime/malloc.go:711
# 0x123456 runtime.newobject+0x42 go/gc/src/runtime/malloc.go:760
The text was updated successfully, but these errors were encountered: