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

Huge performance penalty when using async and sync methods together #5

Closed
blair-ahlquist opened this issue May 17, 2022 · 2 comments
Closed

Comments

@blair-ahlquist
Copy link

The root issue comes from the behavior of SemaphoreSlim. See: this issue.

Is this library still being maintained? Would you entertain pull requests?

@kpreisser
Copy link
Owner

kpreisser commented Jun 9, 2022

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!

@markusschaber
Copy link

markusschaber commented Jun 29, 2022

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.

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

No branches or pull requests

3 participants