[GitHub] tinkerpop pull request #885: TINKERPOP-1978 Gremlin.Net: Add check for conne...
Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/885 ---
[GitHub] tinkerpop pull request #885: TINKERPOP-1978 Gremlin.Net: Add check for conne...
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 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.TryTake`](https://msdn.microsoft.com/en-us/library/dd381774(v=vs.110).aspx)
docs.
---
[GitHub] tinkerpop pull request #885: TINKERPOP-1978 Gremlin.Net: Add check for conne...
Github user dkuppitz commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/885#discussion_r197851424
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs ---
@@ -45,19 +44,28 @@ public ConnectionPool(ConnectionFactory
connectionFactory)
public async Task 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 --
Won't that lead to a never-ending loop? I'm assuming that
`connection.Dispose()` won't remove it from the `_connections` collection, but
I might be wrong.
---
