Botbuilder-dotnet: Semaphore cannot be released in AdalAuthenticator

[chaoteng]

## Version
4.4.3

Describe the bug

The bot is running SDK Microsoft.Bot.Builder 4.4.3 in a Linux docker env of base image dotnet:2.1-runtime.

We see the semaphore error SemaphoreFullException with error message "Adding the specified count to the semaphore would cause it to exceed its maximum count", which suggests semaphore is released more than acquired. From the code in AdalAuthenticator.fs, we do not see the reason how it can happen. Moreover we pulled the botbuilder source code into our application and added some logging around the AdalAuthenticator code to do a test. When the issue happened, semaphore was acquired successfully by waitOne() and when Release() function was called the first time after acquired, it failed and went to retry.

The issue happened mostly after a production deployment. We guess it's because of a spike of the traffic. However, we could not reliably reproduce the issue.

We also tried to replace Semaphore with SemaphoreSlim since the later doesn't use system calls, which might be the issue. After we used SemaphoreSlim, we cannot reproduce the issue anymore but it doesn't mean it's fixed since we could not reliably reproduce the issue before as I said.

I'd like to get some thoughts from community or anyone else has experienced the same issue. Should we change it to SemaphoreSlim since it is supposed to be more suitable for the use case? I know @carlosscastro has made a lot of testings when submitted the PR https://github.com/microsoft/botbuilder-dotnet/pull/1299. Do you have any suggestions on this? Thanks

[bug]

[carlosscastro]

Thanks for reporting this @chaoteng, and for the thorough description! Let me take a look and analyze under which conditions this might happen.

[carlosscastro]

Hello @chaoteng! We are doing the final testing on our fix and will have a pull request tomorrow. We'll send more updates as we make progress, but this should be solved very soon!

[chaoteng]

@carlosscastro Thank you.

[carlosscastro]

@chaoteng, another update, we'll have this be released early next week. We'll try to expedite as much as possible. We changed to semaphoreslim, and we additionally added a few extra layers of protection and error handling, plus added logging to this area which currently does not have any, so for future issues we can diagnose and detect similar issues this much faster. Finally, we also created a new load test to make sure we don’t hit this again after making changes to the code.

Thanks for your patience, and we'll update you one this gets released in a few days.

[chaoteng]

@carlosscastro Thanks for the effort. Looking forward to the pull request.

May I know if your team can reproduce the issue or be able to figure out what is the root cause on .net core level?

[cleemullins]

This has been released earlier this week as part of the C# 4.5.2 SDK.

Please re-open if the issue persists.