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: ReadMemStats fatal error: mappedReady and other memstats are not equal [1.21 backport] #64410

Closed
gopherbot opened this issue Nov 27, 2023 · 4 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone

Comments

@gopherbot
Copy link

@mknyszek requested issue #64401 to be considered for backport to the next 1.21 minor release.

@gopherbot Please open backport issues for Go 1.20 and Go 1.21.

I think it should just be backported. This can cause crashes in correct programs without a workaround, and the fix is very safe.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Nov 27, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 27, 2023
@gopherbot gopherbot added this to the Go1.21.5 milestone Nov 27, 2023
@gopherbot
Copy link
Author

Change https://go.dev/cl/545557 mentions this issue: [release-branch.go1.21] runtime: put ReadMemStats debug assertions behind a double-check mode

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Nov 29, 2023
@gopherbot gopherbot modified the milestones: Go1.21.5, Go1.21.6 Dec 5, 2023
@mdempsky
Copy link
Member

mdempsky commented Jan 3, 2024

Ping, what's the status of this backport?

@mknyszek
Copy link
Contributor

mknyszek commented Jan 3, 2024

I think it's just been waiting to land. I rebased the CL and re-ran trybots.

@gopherbot
Copy link
Author

Closed by merging 4381820 to release-branch.go1.21.

gopherbot pushed a commit that referenced this issue Jan 4, 2024
…hind a double-check mode

ReadMemStats has a few assertions it makes about the consistency of the
stats it's about to produce. Specifically, how those stats line up with
runtime-internal stats. These checks are generally useful, but crashing
just because some stats are wrong is a heavy price to pay.

For a long time this wasn't a problem, but very recently it became a
real problem. It turns out that there's real benign skew that can happen
wherein sysmon (which doesn't synchronize with a STW) generates a trace
event when tracing is enabled, and may mutate some stats while
ReadMemStats is running its checks.

Fix this by synchronizing with both sysmon and the tracer. This is a bit
heavy-handed, but better that than false positives.

Also, put the checks behind a debug mode. We want to reduce the risk of
backporting this change, and again, it's not great to crash just because
user-facing stats are off. Still, enable this debug mode during the
runtime tests so we don't lose quite as much coverage from disabling
these checks by default.

For #64401.
Fixes #64410.

Change-Id: I9adb3e5c7161d207648d07373a11da8a5f0fda9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/545277
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
(cherry picked from commit b2efd1d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/545557
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

4 participants