[sc-53211]

Background

While converting Halibut over to async, we found that the connection manager and connection pool disposes connections while taking new ones. These are async operations, and as such, should be made async.

Results

Before

Connections were being disposed of synchronously. This is IO that would block any thread while disposable occurred.

After

All methods that result in disposal of a connection within the ConnectionPool now has an async equivalent.

In the ConnectionManager, moving from a lock to a SemaphoreSlim introduced a deadlock. This all revolved around the lock in OnConnectionDisposed. This was focused around access to the activeConnections, so we rearranged the locks to ensure access to activeConnections would not result in a deadlock, and introduced a new lock for general access to the connections in general.

ConnectionManagerFixture

The ConnectionManagerFixture tests had a very convoluted setup, with a mixture of mock and real objects. The real objects did not actually do anything related to the test. This included the MessageExchangeProtocol created by GetProtocol. To show how unimportant these 'real' connections were, there was a bug where the async version of EstablishNewConnection was never configured, and all async tests were working fine with an NSubstitute version. We also needed to interrogate the connection, so this whole area was simplified.

To help identify the deadlocks mentioned above, extra test cases were added to ConnectionManagerFixture that caused the deadlock when using SemaphoreSlim as a pure replacement.

Later

Some more work has to be done in regards to async disposal. But that will be done in the async disposal story.

How to review this PR

RequestCancellationTokens.InProgressRequestCancellationToken was introduced while this was being worked on. I believe this is the correct cancellation token to use (as it is used in the same methods). But please verify that my choice was correct 😁

Quality :heavy_check_mark:

Pre-requisites

[ ] I have read How we use GitHub Issues for help deciding when and where it's appropriate to make an issue.
[ ] I have considered informing or consulting the right people, according to the ownership map.
[ ] I have considered appropriate testing for my change.

OctopusDeploy / Halibut

Connection pool async #425