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: loop over allp causes a nil pointer dereference crash #23098
Comments
The failures are all in programs that call |
That loop is concerning because it runs (somewhat non-obviously) without a P, so it could be running concurrently with a STW. I'm not sure how that would be a problem in that particular test, since, as you point out, allp should never be changing after the initial I have a fix, which I will send shortly. I can't reproduce this issue, so it's hard to say if this actually fixes the problem, but it's certainly not a bad thing to fix. |
Change https://golang.org/cl/86215 mentions this issue: |
findrunnable loops over allp to check run queues *after* it has dropped its own P. This is unsafe because allp can change when nothing is blocking safe-points. Hence, procresize could change allp concurrently with findrunnable's loop. Beyond generally violating Go's memory model, in the best case this could findrunnable to observe a nil P pointer if allp has been grown but the new slots not yet initialized. In the worst case, the reads of allp could tear, causing findrunnable to read a word that isn't even a valid *P pointer. Fix this by taking a snapshot of the allp slice header (but not the backing store) before findrunnable drops its P and iterating over this snapshot. The actual contents of allp are immutable up to len(allp), so this fixes the race. Updates #23098 (may fix). Change-Id: I556ae2dbfffe9fe4a1bf43126e930b9e5c240ea8 Reviewed-on: https://go-review.googlesource.com/86215 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
I'm troubled by https://build.golang.org/log/71e1620c62a2255007ea450fc02e9c5d6f5af068 which shows a crash in a different part of |
Several of these seem to be
proc.go:2349 is
It's hard to see a |
I grep'd over the logs since June 29th, and I couldn't find any new instances of this bug. That is, specifically I'll bisect to try to figure out what commit "fixed" it for linux-mipsle. With that said though, should this still be considered a release blocker? |
If it hasn't happened since June 29 I think we can close this. |
OK! I just took a second, slow look over the logs and I'm pretty confident in my assessment. I'll close this bug off. |
See https://build.golang.org/log/ff2e3f95d35cbaec0291bf387a6409c1f71e417f
Seems like there is an exception to the assumption that "none of allp[i] are nil or in state _Pdead." in http://golang.org/cl/45575.
Hej Hej, @aclements
The text was updated successfully, but these errors were encountered: