proc: fix TestCondBreakpointWithFrame flakes on 1.22rc1 #3624
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The flake manifests as an error where the variable i can not be found in frame 1 and happens in go1.22rc1 between 0.1% and 0.5% of the time (it is highly dependent on CPU contention) This problem is caused by the new code in evalop.PushLocal referencing the stale value of SelectedGoroutine. This happens because: - evalop.PushLocal calls ConvertEvalScope - ConvertEvalScope calls FindGoroutine - FindGoroutine checks the value of selectedGoroutine When breakpoint conditions are evaluated both the value of selectedGoroutine and currentThread are stale because we can only set their definitive value *after* all breakpoint conditions have been evaluated. The fact that it only happens in 1.22rc1 is coincidental, it's probably caused by the fact that 1.22rc1 migrates goroutines between threads more in this particular circumstance. This commit fixes the problem in two ways: 1. selectedGoroutine and currentThread are given temprorary non-stale values before breakpoint conditions are evaluated 2. evalop.PushLocal is changed so it takes a stack trace of the current thread rather than resolving it through the selected goroutine. Either one would suffice however I think we should do both, (2) ensures that the runtime.frame(n).var will work even if the current thread is not running any goroutine and (1) ensures that we don't accidentally reference a stale selectedGoroutine in the future.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The flake manifests as an error where the variable i can not be found in
frame 1 and happens in go1.22rc1 between 0.1% and 0.5% of the time (it is highly dependent on CPU contention)
This problem is caused by the new code in evalop.PushLocal referencing the
stale value of SelectedGoroutine. This happens because:
When breakpoint conditions are evaluated both the value of selectedGoroutine
and currentThread are stale because we can only set their definitive value
after all breakpoint conditions have been evaluated.
The fact that it only happens in 1.22rc1 is coincidental, it's probably
caused by the fact that 1.22rc1 migrates goroutines between threads more in
this particular circumstance.
This commit fixes the problem in two ways:
before breakpoint conditions are evaluated
thread rather than resolving it through the selected goroutine.
Either one would suffice however I think we should do both, (2) ensures that
the runtime.frame(n).var will work even if the current thread is not running
any goroutine and (1) ensures that we don't accidentally reference a stale
selectedGoroutine in the future.