search

MichaCo / CacheManager

CacheManager is an open source caching abstraction layer for .NET written in C#. It supports various cache providers and implements many advanced features.
http://cachemanager.michaco.net
Apache License 2.0
2.35k stars 456 forks source link

Avoid using async functions from synchronous code #302

Open pergardebrink opened 4 years ago

pergardebrink commented 4 years ago

This changes the retrytimeout to use Thread.Sleep as we cannot use Task.Delay in this context due to risk of deadlocks.

MichaCo commented 4 years ago

I don't think it is a good idea to use Thread.Sleep. That would suspend the thread which might run many tasks in the host application and could cause even more harm.

I think I'd either use Task.Delay(x).GetAwaiter().GetResult() or remove the delay between retries all together because of possibly too many side effects.

pergardebrink commented 4 years ago

I think I'd either use Task.Delay(x).GetAwaiter().GetResult() or remove the delay between retries all together because of possibly too many side effects.

Wouldn't that also block the current thread in the same way as a Thread.Sleep would do, as the GetResult() call is blocking the current thread until the Task ends?

pergardebrink commented 4 years ago

I think I'd either use Task.Delay(x).GetAwaiter().GetResult() or remove the delay between retries all together because of possibly too many side effects.

Wouldn't that also block the current thread in the same way as a Thread.Sleep would do, as the GetResult() call is blocking the current thread until the Task ends?

Looked a bit more into this for a reference for my reasoning here. David Fowler at Microsoft has a great collection on guidance on async programming. He has a section on "sync over async" about that invoking an async method (for example Task.Delay) and then blocking the thread while waiting (GetAwaiter().GetResult()) for it to complete could be worse than just calling the synchronous version (Thread.Sleep): https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#warning-sync-over-async