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: TestSelectStackAdjust "failed to trigger concurrent GC" #44610

Closed
bcmills opened this issue Feb 25, 2021 · 11 comments
Closed

runtime: TestSelectStackAdjust "failed to trigger concurrent GC" #44610

bcmills opened this issue Feb 25, 2021 · 11 comments

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented Feb 25, 2021

2021-02-25T04:55:35-666ad85/windows-amd64-2012

##### GOMAXPROCS=2 runtime -cpu=1,2,4 -quick
--- FAIL: TestSelectStackAdjust (0.02s)
    chan_test.go:780: failed to trigger concurrent GC
FAIL
FAIL	runtime	39.091s

This is the exact same failure mode as in #34693, which was believed to be fixed in December 2019. This is the first such failure in the logs since CL 210217, which suggests either a recent regression in the runtime (perhaps the GC pacer?) or a lingering very-low-probability flake in the test.

CC @mknyszek @aclements @ianlancetaylor

@bcmills
Copy link
Member Author

@bcmills bcmills commented Feb 25, 2021

Tentatively marking as release-blocker for Go 1.17 until we can determine whether this is actually a regression.

Loading

@bcmills bcmills added this to the Go1.17 milestone Feb 25, 2021
@aclements
Copy link
Member

@aclements aclements commented Mar 18, 2021

FYI, we had this failure a lot in toward the end of the 2019, starting with 2019-09-04T17:56:17-0607cdd and ending with 2019-12-05T22:08:26-e751af1, mostly on openbsd-arm though also on a smattering of other platforms. The one failure @bcmills linked is the only other time we've seen this.

Loading

@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Apr 8, 2021

There's a new occurrence today.

Loading

@aclements
Copy link
Member

@aclements aclements commented Apr 8, 2021

Loading

@toothrot
Copy link
Contributor

@toothrot toothrot commented Apr 15, 2021

@cherrymui @mknyszek Could either of you help investigating this issue? It's a release-blocker for Go 1.17.

Loading

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Apr 15, 2021

I'll look into it next week unless Cherry gets to it first.

Loading

@mknyszek mknyszek self-assigned this Apr 15, 2021
@toothrot
Copy link
Contributor

@toothrot toothrot commented Apr 22, 2021

@mknyszek Friendly ping, as this is marked as a release-blocker.

Loading

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Apr 22, 2021

Sorry for the delay. :( I've been somewhat busy with other bugs. It's on my list.

Loading

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Apr 23, 2021

I haven't been able to reproduce yet, but this test seems rather unstable to me.

I don't think there's necessarily a GC pacing bug, there's a lot that could be going wrong here. For instance, if a test runs just before this with a large-ish heap (we have some tests that do this, O(100 MiB), but 50 MiB will do), and doesn't clean up by running runtime.GC a few times then this could easily fail. It's possible that what happened on Windows is that its set of tests results in a different ordering compared to other platforms, hence the failure.

But more fundamentally, if the goal of the test is to just trigger a concurrent GC, why try to do so indirectly? We have runtime.GC and the stacks are going to get shrunk there too. That completely avoids this failure.

I think what I want to do here is just switch the test to calling runtime.GC directly. I'll try to induce a bug in stack shrinking to make sure it actually triggers.

Loading

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Apr 23, 2021

Just for fun, I came up with a proof of concept: if you get unlucky and the last test to run had a big heap goal and a lot of runway left behind, it's not too hard to induce the failure.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 23, 2021

Change https://golang.org/cl/313109 mentions this issue: runtime: simplify TestSelectStackAdjust

Loading

@gopherbot gopherbot closed this in e7db792 Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants