benaadams / Ben.BlockingDetector

Blocking Detection for ASP.NET Core
Apache License 2.0
731 stars 35 forks source link

SynchronizationContext improperly restored #31

Open OwnageIsMagic opened 1 year ago

OwnageIsMagic commented 1 year ago

https://github.com/benaadams/Ben.BlockingDetector/blob/fce6c534dfcee6ba8cb3ef74d246211b81f5cbeb/src/Ben.BlockingDetector/BlockingDetectionMiddleware.cs#L31-L42 SetSynchronizationContext after awaiting on task restores context on continuation thread (not caller). See https://github.com/npgsql/npgsql/issues/1593 https://stackoverflow.com/questions/44418761/cross-thread-exception-after-async-call/44424562#44424562 @benaadams project seems like not maintained, just for your (and others) information.

maxisam commented 2 weeks ago

I don't know if this is true. I think it is a bit different from the SO link you post.

In that SO post, it clear the SynchronizationContext of the original thread, so after await, it could run on a different thread.

It is like using ConfigureAwait(false), which was exactly what npgsql try to achieve

But here we actually give it a context and create one if there is no context.

so after await, it should go back to the original thread.

OwnageIsMagic commented 2 weeks ago

@maxisam SynchronizationContext is thread local And DetectBlockingSynchronizationContext doesn't guarantee await will return to the same thread (it even doesn't override Post/Send methods)

maxisam commented 2 weeks ago

@OwnageIsMagic I guess I am confused about how await work. I thought await always return to the original context and that is tied with the thread. That is why we don't use configureAwait(false) for UI related code, because it will use any available thread from the thread pool. But if we only use await, it will return to the original thread if there is a context involved.