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

runtime: gcAssistAlloc can call itself recursively #12894

Closed
deft-code opened this issue Oct 9, 2015 · 6 comments
Closed

runtime: gcAssistAlloc can call itself recursively #12894

deft-code opened this issue Oct 9, 2015 · 6 comments

Comments

@deft-code
Copy link

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

@ianlancetaylor ianlancetaylor changed the title gcAssistAlloc can call itself recursively runtime: gcAssistAlloc can call itself recursively Oct 10, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.5.2 milestone Oct 10, 2015
@gopherbot
Copy link

CL https://golang.org/cl/15720 mentions this issue.

aclements added a commit that referenced this issue Oct 12, 2015
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>
@ianlancetaylor
Copy link
Contributor

Is this fixed now? In both 1.5 branch and tip?

@aclements
Copy link
Member

Yes. It was fixed on master by 65aa2d6 and on the 1.5 branch by c257dfb.

@aclements
Copy link
Member

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.

@aclements aclements reopened this Oct 15, 2015
@gopherbot
Copy link

CL https://golang.org/cl/15891 mentions this issue.

aclements added a commit that referenced this issue Oct 15, 2015
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>
@aclements
Copy link
Member

Fixed (for real this time) on release branch by 71a7647.

@golang golang locked and limited conversation to collaborators Oct 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants