-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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/pprof: TestGoroutinesCounts is flaky #15156
Comments
Flaky tests are a distraction and cover up real problems. File bugs instead and mark them as flaky. This moves the net/http flaky test flagging mechanism to internal/testenv. Updates #15156 Updates #15157 Updates #15158 Change-Id: I0e561cd2a09c0dec369cd4ed93bc5a2b40233dfe Reviewed-on: https://go-review.googlesource.com/21614 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
CL https://golang.org/cl/42953 mentions this issue. |
It looks like it's still flaky
I suspect it might be related to a busy OS scheduler, I can reproduce it within 500 runs when running the new test using tip when I have ~10 other busy processes on a Linux system. Sometimes the slower goroutine seems to be in or just return from The flakyness goes away (under the same load) with 40 000 runs if I add
@aclements would you like me to add a CL, or is there a better way to synchronize for this test? PS! Looks like profile.String() here has an unrelated issue: |
@chlunde, sure, that seems like a fine change to that test. Please send a CL.
I tried to reproduce this and get a stack using testing/quick, but the only thing it found is that passing a nil *Profile crashes, but that would have crashed the real test earlier. (Unfortunately, testing/quick itself often crashes when it tries to set the value of an unexported field in Profile!) |
CL https://golang.org/cl/43590 mentions this issue. |
@aclements I see there's an uncommited change for the panic on https://go-review.googlesource.com/c/43152/ |
CL https://golang.org/cl/43630 mentions this issue. |
profileBuilder.locForPC returns 0 to mean "no location" because 0 is an invalid location index. However, the code to build count profiles doesn't check the result of locForPC, so this 0 location index ends up in the profile's location list. This, in turn, causes problems later when we decode the profile because it puts a nil *Location in the sample's location slice, which can later lead to a nil pointer panic. Fix this by making printCountProfile correctly discard the result of locForPC if it returns 0. This makes this call match the other two calls of locForPC. Updates #15156. Change-Id: I4492b3652b513448bc56f4cfece4e37da5e42f94 Reviewed-on: https://go-review.googlesource.com/43630 Reviewed-by: Michael Matloob <matloob@golang.org> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
https://storage.googleapis.com/go-build-log/4d227eb8/openbsd-amd64-gce58_b43ea4d4.log
The text was updated successfully, but these errors were encountered: