Skip to content

runtime: "bad sequence number" in consistentHeapStats.acquire #49395

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.

Sign up for GitHub

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

Jump to bottom
Closed
aclements opened this issue Nov 5, 2021 · 2 comments
Closed

runtime: "bad sequence number" in consistentHeapStats.acquire #49395

aclements opened this issue Nov 5, 2021 · 2 comments
Labels
FrozenDueToAge release-blocker
Milestone
Go1.18

Comments

@aclements
Copy link
Member

It's currently possible for a poorly timed stack split in runtime.(*consistentHeapStats).release to lead to a fatal "bad sequence number". The exact steps are:

  1. mcache.refill calls consistentHeapStats.acquire
  2. mcache.refill updates heap stats
  3. mcache.refill calls consistentHeapStats.release
  4. The prologue of consistentHeapStats.release performs a stack split before the release actually happens
  5. This enters the stack allocator, which eventually calls consistentHeapStats.acquire

This is effectively a "double acquire", which causes acquire to panic with "bad sequence number".

I found this using the maymorestack tests added in CL 361212. @mknyszek already has a fix.
@aclements aclements added this to the Go1.18 milestone Nov 5, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/361158 mentions this issue: runtime: make consistentHeapStats acquire/release nosplit

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/361212 mentions this issue: cmd/dist: add maymorestack tests

gopherbot pushed a commit that referenced this issue Apr 25, 2022
@aclements @gopherbot
cmd/dist: add maymorestack tests
60178e6 
These tests run the runtime, reflect, and sync package tests with the
two maymorestack hooks we have.

These tests only run on the longtest builders (or with
GO_TEST_SHORT=false) because we're running the runtime test two
additional times and the mayMoreStackMove hook makes it about twice as
slow (~230 seconds).

To run just these tests by hand, do

  GO_TEST_SHORT=false go tool dist test -run mayMoreStack

Updates #48297.

This detected #49354, which was found as a flake on the dashboard, but
was reliably reproducible with these tests; and #49395.

Change-Id: If785a8b8d6e1b9ad4d2ae67493b54055ab6cbc85
Reviewed-on: https://go-review.googlesource.com/c/go/+/361212
Run-TryBot: Austin Clements <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Austin Clements <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
3 participants
@mknyszek @aclements @gopherbot