From 98e4e1b8201f50e1f34458a82c20f2737aa6ae53 Mon Sep 17 00:00:00 2001 From: florianhockmann Date: Sat, 2 Sep 2017 17:54:39 +0200 Subject: [PATCH 1/3] Add handling for closed connections Closed connections are not returned to the ConnectionPool as the client cannot send any further requests with them. Additionally, the host is considered to be unavailable when a connection was closed and all other connections are closed and removed from the ConnectionPool. This is in line with the handling for closed connection in gremlin-driver. --- .../src/Gremlin.Net/Driver/Connection.cs | 2 + .../src/Gremlin.Net/Driver/ConnectionPool.cs | 62 +++++++++++++------ .../Gremlin.Net/Driver/WebSocketConnection.cs | 2 + 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs index 126b46117c4..dbbd375f9ee 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs @@ -68,6 +68,8 @@ public async Task CloseAsync() await _webSocketConnection.CloseAsync().ConfigureAwait(false); } + public bool IsOpen => _webSocketConnection.IsOpen; + private async Task SendAsync(RequestMessage message) { var graphsonMsg = _graphSONWriter.WriteObject(message); diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs index 945e5e489c4..779815a764c 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs @@ -55,7 +55,7 @@ public async Task GetAvailableConnectionAsync() if (connection == null) connection = await CreateNewConnectionAsync().ConfigureAwait(false); - return new ProxyConnection(connection, AddConnection); + return new ProxyConnection(connection, AddConnectionIfOpen); } private async Task CreateNewConnectionAsync() @@ -66,6 +66,17 @@ private async Task CreateNewConnectionAsync() return newConnection; } + private void AddConnectionIfOpen(Connection connection) + { + if (!connection.IsOpen) + { + ConsiderUnavailable(); + connection.Dispose(); + return; + } + AddConnection(connection); + } + private void AddConnection(Connection connection) { lock (_connectionsLock) @@ -74,6 +85,36 @@ private void AddConnection(Connection connection) } } + private void ConsiderUnavailable() + { + CloseAndRemoveAllConnections(); + } + + private void CloseAndRemoveAllConnections() + { + lock (_connectionsLock) + { + TeardownAsync().WaitUnwrap(); + RemoveAllConnections(); + } + } + + private void RemoveAllConnections() + { + while (!_connections.IsEmpty) + { + _connections.TryTake(out var connection); + connection.Dispose(); + } + } + + private async Task TeardownAsync() + { + var closeTasks = new List(_connections.Count); + closeTasks.AddRange(_connections.Select(conn => conn.CloseAsync())); + await Task.WhenAll(closeTasks).ConfigureAwait(false); + } + #region IDisposable Support private bool _disposed; @@ -89,27 +130,10 @@ protected virtual void Dispose(bool disposing) if (!_disposed) { if (disposing) - lock (_connectionsLock) - { - if (_connections != null && !_connections.IsEmpty) - { - TeardownAsync().WaitUnwrap(); - - foreach (var conn in _connections) - conn.Dispose(); - } - } + CloseAndRemoveAllConnections(); _disposed = true; } } - - private async Task TeardownAsync() - { - var closeTasks = new List(_connections.Count); - closeTasks.AddRange(_connections.Select(conn => conn.CloseAsync())); - await Task.WhenAll(closeTasks).ConfigureAwait(false); - } - #endregion } } \ No newline at end of file diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/WebSocketConnection.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/WebSocketConnection.cs index 5a20759763d..9672606ad4b 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/WebSocketConnection.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/WebSocketConnection.cs @@ -71,6 +71,8 @@ public async Task ReceiveMessageAsync() } } + public bool IsOpen => _client.State == WebSocketState.Open; + #region IDisposable Support private bool _disposed; From d9a4f79f7c568761461dc11d22fbf907578f7d14 Mon Sep 17 00:00:00 2001 From: florianhockmann Date: Sat, 2 Sep 2017 18:00:52 +0200 Subject: [PATCH 2/3] Update NuGet packages of test projects One of the dependencies (Castle.Core) produced a warning because one of its dependencies couldn't be found in the specified version. This doesn't occur anymore for the updated version. --- .../Gremlin.Net.IntegrationTest.csproj | 2 +- .../test/Gremlin.Net.UnitTest/Gremlin.Net.UnitTest.csproj | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gremlin.Net.IntegrationTest.csproj b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gremlin.Net.IntegrationTest.csproj index fed1c49ce14..9d00a5ac88a 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gremlin.Net.IntegrationTest.csproj +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gremlin.Net.IntegrationTest.csproj @@ -19,7 +19,7 @@ - + diff --git a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Gremlin.Net.UnitTest.csproj b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Gremlin.Net.UnitTest.csproj index db36085da50..41617bed7f2 100644 --- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Gremlin.Net.UnitTest.csproj +++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Gremlin.Net.UnitTest.csproj @@ -13,8 +13,8 @@ - - + + From ef658752099e127fa1f4e3e6737a0b6f81270622 Mon Sep 17 00:00:00 2001 From: florianhockmann Date: Wed, 6 Sep 2017 20:53:11 +0200 Subject: [PATCH 3/3] Improve closing and removing of connections in ConnectionPool This resolves a possible race condition. --- gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs index 779815a764c..9501686b2aa 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs @@ -101,18 +101,15 @@ private void CloseAndRemoveAllConnections() private void RemoveAllConnections() { - while (!_connections.IsEmpty) + while (_connections.TryTake(out var connection)) { - _connections.TryTake(out var connection); connection.Dispose(); } } private async Task TeardownAsync() { - var closeTasks = new List(_connections.Count); - closeTasks.AddRange(_connections.Select(conn => conn.CloseAsync())); - await Task.WhenAll(closeTasks).ConfigureAwait(false); + await Task.WhenAll(_connections.Select(c => c.CloseAsync())).ConfigureAwait(false); } #region IDisposable Support