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

Change mechanism for losing the synchronization context #528

Conversation

leeoades
Copy link
Contributor

There is a race condition where Task.Delay could finish synchronously. Change to a mechanism that ensures a sync context change.

… synchronously. Change to a mechanism that ensures a sync context change.
@leeoades
Copy link
Contributor Author

I switched to using Task.Yield, and it didn't seem to lose the Sync Context in my testing. CI didn't like it either, so I've reverted to my original Task.Run.

@crozone
Copy link
Collaborator

crozone commented May 2, 2023

Would

System.Threading.SynchronizationContext.SetSynchronizationContext(null);

work here?

@leeoades
Copy link
Contributor Author

leeoades commented May 2, 2023

Hi @crozone
That would do something different - let me explain.

I'm in Sync Context A and I await an async Task.
When that Task has completed, if I have ConfigureAwait(false) then the code continues to execute in Sync Context B.
My test code is simulating this.
The tests prove that my new changes restore the executing code back into Sync Context A.

However, If instead of awaiting an async Task and having it return in Sync Context B, I call SetSynchronizationContext(null); this would change the "main" thread with Sync Context A to have no Sync Context. So this isn't simulating the scenario I've fixed and actually means that I've lost the test Sync Context I'm testing for.

I hope that makes sense. Good suggestion though.

@mclift
Copy link
Member

mclift commented May 10, 2023

Thanks for the follow-up PR, @leeoades, and to @gao-artur for pointing out the issue in #519.

@mclift mclift merged commit e69c66c into dotnet-state-machine:dev May 10, 2023
@crozone
Copy link
Collaborator

crozone commented Nov 15, 2023

We are very occasionally seeing test failures from this code, for example:

https://github.com/dotnet-state-machine/stateless/actions/runs/6823242739/job/18556789583

I've created an issue here: #549

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants