-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Steps to investigate CI flakiness with async-streams tests #58864
Conversation
public ValueTaskSourceStatus GetStatus(short token) | ||
{ | ||
ValidateToken(token); | ||
return | ||
!_completed ? ValueTaskSourceStatus.Pending : | ||
_continuation == null || !_completed ? ValueTaskSourceStatus.Pending : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stephentoub FWIW, this seems the only actual code change when updating ManualResetValueTaskSourceCore
source code, but I'm not seeing how it could relate to the symptom (ie. the completion sentinel being invoked, see stacktrace below).
Although almost all the failures occur on Linux, there's one on Windows too (so not exclusive).
at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCoreShared.ThrowInvalidOperationException()
at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCoreShared.CompletionSentinel(Object _)
at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1.InvokeContinuation()
at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1.SignalCompletion()
at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1.SetResult(TResult result)
at C.<Main>g__local|0_0()
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine)
at System.Runtime.CompilerServices.AsyncIteratorMethodBuilder.MoveNext[TStateMachine](TStateMachine& stateMachine)
at C.<Main>g__local|0_0()
at C.Main()
at C.Main()
at C.<Main>()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That change came from dotnet/coreclr#22711 which was addressing a race condition. So... it might be relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (commit 4)
Merging with a single review since test-only change. Thanks |
From discussion with Stephen, those steps should help:
Relates to #34207
Relates to #58196
Relates to #47156
Relates to #40927