Huge performance penalty when using async and sync methods together
blair-ahlquist opened this issue · 2 comments
The root issue comes from the behavior of SemaphoreSlim. See: this issue.
Is this library still being maintained? Would you entertain pull requests?
Hi @blair-ahlquist,
thanks for create this issue (I'm currently not actively working on this project).
I assume you mean the issue described in your link, that when you call both the sync and async variants of SemaphoreSlim's wait methods Wait()/WaitAsync() in a number of tasks at the same time that are scheduled at worker threads of the .NET ThreadPool, this can cause a quasi deadlock (at least a long hang).
However, IMHO this isn't an actual issue in SemaphoreSlim or .NET, but a wrong usage of these methods (which applies then also to the AsyncReaderWriterLockSlim).
If you call e.g. SemaphoreSlim.WaitAsync() when the semaphore currently isn't available (has Count == 0), and then it is released (and decides that one of the async waiters will acquire it), it will schedule to run the continuation of the returned Task of the async waiter in one of the .NET ThreadPool's worker threads by default.
However, when currently all worker threads are blocked due to calling the synchronous Wait() method (e.g. scheduled with Task.Run(), and there are even more tasks scheduled than worker threads available), no thread in the thread pool is available any more, and therefore it can take a long time until the application continues (because the thread pool is creating new threads slowly, AFAIK about 1 thread per second, and eventually there will be enough new threads so that one of them can run the async continuation and run the code that eventually releases the semaphore again, which in turn would then also allow the synchronous waiters to acquire it).
The same can happen in such a case if you only use the synchronous Wait() method, but execute async code between Wait() and Release() (so a thread switch may happen), as then in such a case there may no worker thread be available in order to run the continuation (that would eventually release the semaphore).
To solve this, you should either only use the async variants (WaitAsync()) in worker threads of the .NET ThreadPool, and use the synchronous variants (Wait()) only in dedicated threads (e.g. that are created with new Thread(...)), as then there will no worker threads be blocked by synchronous wait calls.
Or, you should always only use the sync wait methods, and don't run async code between Wait() and Release(), because then it can't happen that a thread switch happens between acquiring and releasing the semaphore. (In this case you could use the regular, non-async System.Threading.ReaderWriterLockSlim instead.)
Therefore, I don't think there is anything that can/should be done about this in the AsyncReaderWriterLockSlim.
Thanks!
The AsyncEx package from @StephenCleary has similar problems, the proposed solution is some dedicated "completer Thread" (but apparently not implemented yet).
See StephenCleary/AsyncEx#107 and https://ayende.com/blog/177953/thread-pool-starvation-just-add-another-thread for more information.