Skip to content

runtime/race: tests stop early due to "range function continued iteration after loop body panic" #72925

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

Open
neild opened this issue Mar 18, 2025 · 8 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@neild
Copy link
Contributor

neild commented Mar 18, 2025

runtime/race.TestRace runs a collection of tests in runtime/race/testdata, with the expectation that some pass and some fail due to data races.

TestRaceRangeFuncIterator (https://go.googlesource.com/go/+/refs/tags/go1.24.0/src/runtime/race/testdata/rangefunc_test.go#67) is currently terminating the test run due to a "range function continued iteration after loop body panic" error. Tests after this one don't run.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Mar 18, 2025
@randall77
Copy link
Contributor

Should we change runtime.panicrangestate to do something different when the race detector is on?
(Report a "race" instead of panicking?)

@neild
Copy link
Contributor Author

neild commented Mar 18, 2025

Related: TestNoRaceRangeFuncIterator (the next test in line) fails with an unexpected data race. The failure wasn't showing up due to the previous test terminating the entire run.

@neild
Copy link
Contributor Author

neild commented Mar 18, 2025

I think the race detector isn't relevant to the panic (although I might be mistaken).

These tests range over a func that starts two goroutines which simultaneously yield values. The "range func continued iteration after loop body panic" happens when the range loop breaks early.

This seems to happen even if the loop body doesn't panic. I think we're incorrectly producing an "...after loop body panic" error when it should be "...after loop body exited".

We also seem to be detecting a data race when a rangefunc's yield is called concurrently. TestNoRaceRangeFuncIterator expects this to work, but perhaps the test is out of date?

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/658875 mentions this issue: runtime/race: detect when TestRace fails to run all tests, skip failures

@AndrewHarrisSPU
Copy link

We also seem to be detecting a data race when a rangefunc's yield is called concurrently. TestNoRaceRangeFuncIterator expects this to work, but perhaps the test is out of date?

It does seem like splicing the linked list of defers involved in a range function could be racy in the general circumstance? I'm probably well out of my depth and not sure if this is what's detected by the race detector, but, like map writes, detecting the race is a little different from observing an unexpected outcome ... in this case I think it's fair to observe that yield is being raced.

gopherbot pushed a commit that referenced this issue Mar 19, 2025
TestRace runs a collection of tests, some of which are expected
to fail with data races. Make TestRace more robust at detecting
when the test run is cut short, such as when a test causes
an unhandled panic.

Skip TestRaceRangeFuncIterator, which contains an unhandled panic.
This test was causing all subsequent tests to not run.

Skip TestNoRaceRangeFuncIterator, which contains an unexpected data race.
This test was not running due to the above failure.

For #72925

Change-Id: Id662375cc498ea25ae308619709768588bf6a2f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/658875
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
@mknyszek
Copy link
Contributor

CC @dr2chase

@mknyszek
Copy link
Contributor

In triage, we think we need to leave this bug open until we can re-enable the tests.

@mknyszek mknyszek added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 19, 2025
@mknyszek mknyszek added this to the Go1.25 milestone Mar 19, 2025
@dr2chase
Copy link
Contributor

Why not fix the test to catch the panic and continue? The rangefunc checking are designed to (often) blow up if the yield function is run in parallel. I.e., this is working-as-intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

No branches or pull requests

6 participants