search

dotnet-state-machine / stateless

A simple library for creating state machines in C# code
Other
5.53k stars 765 forks source link

Change mechanism for losing the synchronization context #528

Closed leeoades closed 1 year ago

leeoades commented 1 year ago

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

leeoades commented 1 year ago

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 commented 1 year ago

Would

System.Threading.SynchronizationContext.SetSynchronizationContext(null);

work here?

leeoades commented 1 year ago

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 commented 1 year ago

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

crozone commented 10 months ago

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: https://github.com/dotnet-state-machine/stateless/issues/549