Lock in OnlineClientManager was flagged by Microsoft support as contributing to deadlock in high volume traffic scenarious

[SergeiGorlovetsky]

Hi,
One of our clients has been experiencing Thread Exhaustion issues and after analysing memory dumps and .net profiler traces the recommendation was:
" As with most performance issues, the scene is complex. However, the most prominent issue is definitely the huge number of blocked threads that are all waiting on the synclock to either call Abp.RealTime.OnlineClientManager.Add or Abp.RealTime.OnlineClientManager.Remove. The application cannot get that lock because another thread has the lock and it’s itself blocked on the lock for another object, a System.Collections.Concurrent.BlockingCollection in Nito.AsyncEx.AsyncContext. Note none of these are Microsoft libraries, what was found matches a known issue talked about here: Deadlock with AsyncReaderWriterLock · Issue #255 · StephenCleary/AsyncEx (github.com) as well as here: Another deadlock - or at least a very slow lock · Issue #107 · StephenCleary/AsyncEx (github.com)
To try and make this accessible: Under load, the application needs to have free threads in order to unlock the lock on the AsyncContext::TaskQueue. If the threadpool is exhausted, then best case the thread will block for 2 seconds until a new thread is added to the pool. Under load, that thread might be assigned to other duties! The other duties though just end up leading to calls to the first locked object (OnlineClientManager) where they block, adding to the thread pool exhaustion. "
I've noticed a note a TODO note in OnlineClientManager code suggesting to remove SyncObj as the default store is thread safe.
Please advise of your thoughts on this.

Thanks,
Sergei Gorlovetsky

[ismcagdas]

Hi @SergeiGorlovetsky I think at the time of writing OnlineClientManager, we couldn't be sure about InMemoryOnlineClientStore is thread safe or not but when I look at it, it seems fine. To try this, you can create your own version of OnlineClientManager and use it in your project and test it.

[SergeiGorlovetsky]

Thank you Ismail, Indeed InMemoryOnlineClientStore uses ConcurrentDictionary and it is thread safe. We are going to remove the lock and load test.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ismcagdas]

@SergeiGorlovetsky did you have a chance to test this ?