Github user FlorianHockmann commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/885#discussion_r197855449
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs ---
@@ -45,19 +44,28 @@ public ConnectionPool(ConnectionFactory
connectionFactory)
public async Task<IConnection> GetAvailableConnectionAsync()
{
- Connection connection = null;
- lock (_connectionsLock)
- {
- if (!_connections.IsEmpty)
- _connections.TryTake(out connection);
- }
-
- if (connection == null)
+ if (!TryGetConnectionFromPool(out var connection))
connection = await
CreateNewConnectionAsync().ConfigureAwait(false);
return new ProxyConnection(connection, AddConnectionIfOpen);
}
+ private bool TryGetConnectionFromPool(out Connection connection)
+ {
+ while (true)
+ {
+ connection = null;
+ lock (_connectionsLock)
+ {
+ if (_connections.IsEmpty) return false;
+ _connections.TryTake(out connection);
+ }
+
+ if (connection.IsOpen) return true;
+ connection.Dispose();
--- End diff --
`_connections.TryTake(out connection)` removes the connection from the
collection. So, here it is already no longer part of `_connections`. That may
be a bit confusing as `_connections` doesn't contain all connections in the
pool but only those that are currently unused. That made this class a lot
easier to implement as no checks are needed of whether a connection is already
in use or not.
For reference: The
[`ConcurrentBag<T>.TryTake`](https://msdn.microsoft.com/en-us/library/dd381774(v=vs.110).aspx)
docs.
---
