diff --git a/doc/connection-backoff-interop-test-description.md b/doc/connection-backoff-interop-test-description.md
index 0f00c86dca2..64405431d2a 100644
--- a/doc/connection-backoff-interop-test-description.md
+++ b/doc/connection-backoff-interop-test-description.md
@@ -31,9 +31,9 @@ Clients should accept these arguments:
* --server_retry_port=PORT
* The server port to connect to for testing backoffs. For example, "8081"
-The client must connect to the control port without TLS. The client should
-either assert on the server returned backoff status or check the returned
-backoffs on its own.
+The client must connect to the control port without TLS. The client must connect
+to the retry port with TLS. The client should either assert on the server
+returned backoff status or check the returned backoffs on its own.
Procedure of client:
diff --git a/doc/connection-backoff.md b/doc/connection-backoff.md
index 7094e737c51..251a60f384b 100644
--- a/doc/connection-backoff.md
+++ b/doc/connection-backoff.md
@@ -44,3 +44,12 @@ different jitter logic.
Alternate implementations must ensure that connection backoffs started at the
same time disperse, and must not attempt connections substantially more often
than the above algorithm.
+
+## Reset Backoff
+
+The back off should be reset to INITIAL_BACKOFF at some time point, so that the
+reconnecting behavior is consistent no matter the connection is a newly started
+one or a previously disconnected one.
+
+We choose to reset the Backoff when the SETTINGS frame is received, at that time
+point, we know for sure that this connection was accepted by the server.
diff --git a/src/csharp/Grpc.Core.Tests/ChannelTest.cs b/src/csharp/Grpc.Core.Tests/ChannelTest.cs
index 27875729240..dfbd92879e7 100644
--- a/src/csharp/Grpc.Core.Tests/ChannelTest.cs
+++ b/src/csharp/Grpc.Core.Tests/ChannelTest.cs
@@ -41,12 +41,6 @@ namespace Grpc.Core.Tests
{
public class ChannelTest
{
- [TestFixtureTearDown]
- public void CleanupClass()
- {
- GrpcEnvironment.Shutdown();
- }
-
[Test]
public void Constructor_RejectsInvalidParams()
{
@@ -56,36 +50,33 @@ namespace Grpc.Core.Tests
[Test]
public void State_IdleAfterCreation()
{
- using (var channel = new Channel("localhost", Credentials.Insecure))
- {
- Assert.AreEqual(ChannelState.Idle, channel.State);
- }
+ var channel = new Channel("localhost", Credentials.Insecure);
+ Assert.AreEqual(ChannelState.Idle, channel.State);
+ channel.ShutdownAsync().Wait();
}
[Test]
public void WaitForStateChangedAsync_InvalidArgument()
{
- using (var channel = new Channel("localhost", Credentials.Insecure))
- {
- Assert.Throws(typeof(ArgumentException), () => channel.WaitForStateChangedAsync(ChannelState.FatalFailure));
- }
+ var channel = new Channel("localhost", Credentials.Insecure);
+ Assert.Throws(typeof(ArgumentException), () => channel.WaitForStateChangedAsync(ChannelState.FatalFailure));
+ channel.ShutdownAsync().Wait();
}
[Test]
public void ResolvedTarget()
{
- using (var channel = new Channel("127.0.0.1", Credentials.Insecure))
- {
- Assert.IsTrue(channel.ResolvedTarget.Contains("127.0.0.1"));
- }
+ var channel = new Channel("127.0.0.1", Credentials.Insecure);
+ Assert.IsTrue(channel.ResolvedTarget.Contains("127.0.0.1"));
+ channel.ShutdownAsync().Wait();
}
[Test]
- public void Dispose_IsIdempotent()
+ public void Shutdown_AllowedOnlyOnce()
{
var channel = new Channel("localhost", Credentials.Insecure);
- channel.Dispose();
- channel.Dispose();
+ channel.ShutdownAsync().Wait();
+ Assert.Throws(typeof(InvalidOperationException), () => channel.ShutdownAsync().GetAwaiter().GetResult());
}
}
}
diff --git a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs
index e49fdb5268c..68279a20076 100644
--- a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs
+++ b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs
@@ -63,16 +63,10 @@ namespace Grpc.Core.Tests
[TearDown]
public void Cleanup()
{
- channel.Dispose();
+ channel.ShutdownAsync().Wait();
server.ShutdownAsync().Wait();
}
- [TestFixtureTearDown]
- public void CleanupClass()
- {
- GrpcEnvironment.Shutdown();
- }
-
[Test]
public async Task UnaryCall()
{
@@ -207,13 +201,6 @@ namespace Grpc.Core.Tests
CollectionAssert.AreEqual(headers[1].ValueBytes, trailers[1].ValueBytes);
}
- [Test]
- public void UnaryCall_DisposedChannel()
- {
- channel.Dispose();
- Assert.Throws(typeof(ObjectDisposedException), () => Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "ABC"));
- }
-
[Test]
public void UnaryCallPerformance()
{
diff --git a/src/csharp/Grpc.Core.Tests/CompressionTest.cs b/src/csharp/Grpc.Core.Tests/CompressionTest.cs
index 9547683f60f..378c81851c0 100644
--- a/src/csharp/Grpc.Core.Tests/CompressionTest.cs
+++ b/src/csharp/Grpc.Core.Tests/CompressionTest.cs
@@ -62,16 +62,10 @@ namespace Grpc.Core.Tests
[TearDown]
public void Cleanup()
{
- channel.Dispose();
+ channel.ShutdownAsync().Wait();
server.ShutdownAsync().Wait();
}
- [TestFixtureTearDown]
- public void CleanupClass()
- {
- GrpcEnvironment.Shutdown();
- }
-
[Test]
public void WriteOptions_Unary()
{
diff --git a/src/csharp/Grpc.Core.Tests/ContextPropagationTest.cs b/src/csharp/Grpc.Core.Tests/ContextPropagationTest.cs
index db5f953b0e1..2db3f286f7a 100644
--- a/src/csharp/Grpc.Core.Tests/ContextPropagationTest.cs
+++ b/src/csharp/Grpc.Core.Tests/ContextPropagationTest.cs
@@ -62,16 +62,10 @@ namespace Grpc.Core.Tests
[TearDown]
public void Cleanup()
{
- channel.Dispose();
+ channel.ShutdownAsync().Wait();
server.ShutdownAsync().Wait();
}
- [TestFixtureTearDown]
- public void CleanupClass()
- {
- GrpcEnvironment.Shutdown();
- }
-
[Test]
public async Task PropagateCancellation()
{
diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj
index 829effc9a22..ad4e94a6959 100644
--- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj
+++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj
@@ -64,6 +64,7 @@
Version.cs
+
diff --git a/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs b/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs
index 4ed93c7eca2..4fdfab5a994 100644
--- a/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs
+++ b/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs
@@ -43,33 +43,39 @@ namespace Grpc.Core.Tests
[Test]
public void InitializeAndShutdownGrpcEnvironment()
{
- var env = GrpcEnvironment.GetInstance();
+ var env = GrpcEnvironment.AddRef();
Assert.IsNotNull(env.CompletionQueue);
- GrpcEnvironment.Shutdown();
+ GrpcEnvironment.Release();
}
[Test]
public void SubsequentInvocations()
{
- var env1 = GrpcEnvironment.GetInstance();
- var env2 = GrpcEnvironment.GetInstance();
+ var env1 = GrpcEnvironment.AddRef();
+ var env2 = GrpcEnvironment.AddRef();
Assert.IsTrue(object.ReferenceEquals(env1, env2));
- GrpcEnvironment.Shutdown();
- GrpcEnvironment.Shutdown();
+ GrpcEnvironment.Release();
+ GrpcEnvironment.Release();
}
[Test]
public void InitializeAfterShutdown()
{
- var env1 = GrpcEnvironment.GetInstance();
- GrpcEnvironment.Shutdown();
+ var env1 = GrpcEnvironment.AddRef();
+ GrpcEnvironment.Release();
- var env2 = GrpcEnvironment.GetInstance();
- GrpcEnvironment.Shutdown();
+ var env2 = GrpcEnvironment.AddRef();
+ GrpcEnvironment.Release();
Assert.IsFalse(object.ReferenceEquals(env1, env2));
}
+ [Test]
+ public void ReleaseWithoutAddRef()
+ {
+ Assert.Throws(typeof(InvalidOperationException), () => GrpcEnvironment.Release());
+ }
+
[Test]
public void GetCoreVersionString()
{
diff --git a/src/csharp/Grpc.Core.Tests/ResponseHeadersTest.cs b/src/csharp/Grpc.Core.Tests/ResponseHeadersTest.cs
index 981b8ea3c8e..706006702e5 100644
--- a/src/csharp/Grpc.Core.Tests/ResponseHeadersTest.cs
+++ b/src/csharp/Grpc.Core.Tests/ResponseHeadersTest.cs
@@ -69,16 +69,10 @@ namespace Grpc.Core.Tests
[TearDown]
public void Cleanup()
{
- channel.Dispose();
+ channel.ShutdownAsync().Wait();
server.ShutdownAsync().Wait();
}
- [TestFixtureTearDown]
- public void CleanupClass()
- {
- GrpcEnvironment.Shutdown();
- }
-
[Test]
public void WriteResponseHeaders_NullNotAllowed()
{
diff --git a/src/csharp/Grpc.Core.Tests/ServerTest.cs b/src/csharp/Grpc.Core.Tests/ServerTest.cs
index 485006ebac7..e7193c843b9 100644
--- a/src/csharp/Grpc.Core.Tests/ServerTest.cs
+++ b/src/csharp/Grpc.Core.Tests/ServerTest.cs
@@ -51,7 +51,6 @@ namespace Grpc.Core.Tests
};
server.Start();
server.ShutdownAsync().Wait();
- GrpcEnvironment.Shutdown();
}
[Test]
@@ -67,8 +66,7 @@ namespace Grpc.Core.Tests
Assert.Greater(boundPort.BoundPort, 0);
server.Start();
- server.ShutdownAsync();
- GrpcEnvironment.Shutdown();
+ server.ShutdownAsync().Wait();
}
[Test]
@@ -83,7 +81,6 @@ namespace Grpc.Core.Tests
Assert.Throws(typeof(InvalidOperationException), () => server.Services.Add(ServerServiceDefinition.CreateBuilder("serviceName").Build()));
server.ShutdownAsync().Wait();
- GrpcEnvironment.Shutdown();
}
}
}
diff --git a/src/csharp/Grpc.Core.Tests/ShutdownTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownTest.cs
new file mode 100644
index 00000000000..a2be7ddd5e0
--- /dev/null
+++ b/src/csharp/Grpc.Core.Tests/ShutdownTest.cs
@@ -0,0 +1,77 @@
+#region Copyright notice and license
+
+// Copyright 2015, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#endregion
+
+using System;
+using System.Diagnostics;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Grpc.Core;
+using Grpc.Core.Internal;
+using Grpc.Core.Utils;
+using NUnit.Framework;
+
+namespace Grpc.Core.Tests
+{
+ public class ShutdownTest
+ {
+ const string Host = "127.0.0.1";
+
+ MockServiceHelper helper;
+ Server server;
+ Channel channel;
+
+ [SetUp]
+ public void Init()
+ {
+ helper = new MockServiceHelper(Host);
+ server = helper.GetServer();
+ server.Start();
+ channel = helper.GetChannel();
+ }
+
+ [Test]
+ public async Task AbandonedCall()
+ {
+ helper.DuplexStreamingHandler = new DuplexStreamingServerMethod(async (requestStream, responseStream, context) =>
+ {
+ await requestStream.ToListAsync();
+ });
+
+ var call = Calls.AsyncDuplexStreamingCall(helper.CreateDuplexStreamingCall(new CallOptions(deadline: DateTime.UtcNow.AddMilliseconds(1))));
+
+ channel.ShutdownAsync().Wait();
+ server.ShutdownAsync().Wait();
+ }
+ }
+}
diff --git a/src/csharp/Grpc.Core.Tests/TimeoutsTest.cs b/src/csharp/Grpc.Core.Tests/TimeoutsTest.cs
index d875d601b94..41f661f62db 100644
--- a/src/csharp/Grpc.Core.Tests/TimeoutsTest.cs
+++ b/src/csharp/Grpc.Core.Tests/TimeoutsTest.cs
@@ -65,16 +65,10 @@ namespace Grpc.Core.Tests
[TearDown]
public void Cleanup()
{
- channel.Dispose();
+ channel.ShutdownAsync().Wait();
server.ShutdownAsync().Wait();
}
- [TestFixtureTearDown]
- public void CleanupClass()
- {
- GrpcEnvironment.Shutdown();
- }
-
[Test]
public void InfiniteDeadline()
{
diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs
index 64c6adf2bfc..2f8519dfa30 100644
--- a/src/csharp/Grpc.Core/Channel.cs
+++ b/src/csharp/Grpc.Core/Channel.cs
@@ -45,14 +45,19 @@ namespace Grpc.Core
///
/// gRPC Channel
///
- public class Channel : IDisposable
+ public class Channel
{
static readonly ILogger Logger = GrpcEnvironment.Logger.ForType();
+ readonly object myLock = new object();
+ readonly AtomicCounter activeCallCounter = new AtomicCounter();
+
readonly string target;
readonly GrpcEnvironment environment;
readonly ChannelSafeHandle handle;
readonly List options;
+
+ bool shutdownRequested;
bool disposed;
///
@@ -65,7 +70,7 @@ namespace Grpc.Core
public Channel(string target, Credentials credentials, IEnumerable options = null)
{
this.target = Preconditions.CheckNotNull(target, "target");
- this.environment = GrpcEnvironment.GetInstance();
+ this.environment = GrpcEnvironment.AddRef();
this.options = options != null ? new List(options) : new List();
EnsureUserAgentChannelOption(this.options);
@@ -172,12 +177,26 @@ namespace Grpc.Core
}
///
- /// Destroys the underlying channel.
+ /// Waits until there are no more active calls for this channel and then cleans up
+ /// resources used by this channel.
///
- public void Dispose()
+ public async Task ShutdownAsync()
{
- Dispose(true);
- GC.SuppressFinalize(this);
+ lock (myLock)
+ {
+ Preconditions.CheckState(!shutdownRequested);
+ shutdownRequested = true;
+ }
+
+ var activeCallCount = activeCallCounter.Count;
+ if (activeCallCount > 0)
+ {
+ Logger.Warning("Channel shutdown was called but there are still {0} active calls for that channel.", activeCallCount);
+ }
+
+ handle.Dispose();
+
+ await Task.Run(() => GrpcEnvironment.Release());
}
internal ChannelSafeHandle Handle
@@ -196,13 +215,20 @@ namespace Grpc.Core
}
}
- protected virtual void Dispose(bool disposing)
+ internal void AddCallReference(object call)
{
- if (disposing && handle != null && !disposed)
- {
- disposed = true;
- handle.Dispose();
- }
+ activeCallCounter.Increment();
+
+ bool success = false;
+ handle.DangerousAddRef(ref success);
+ Preconditions.CheckState(success);
+ }
+
+ internal void RemoveCallReference(object call)
+ {
+ handle.DangerousRelease();
+
+ activeCallCounter.Decrement();
}
private static void EnsureUserAgentChannelOption(List options)
diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs
index 30d8c802355..0a44eead74a 100644
--- a/src/csharp/Grpc.Core/GrpcEnvironment.cs
+++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs
@@ -58,6 +58,7 @@ namespace Grpc.Core
static object staticLock = new object();
static GrpcEnvironment instance;
+ static int refCount;
static ILogger logger = new ConsoleLogger();
@@ -67,13 +68,14 @@ namespace Grpc.Core
bool isClosed;
///
- /// Returns an instance of initialized gRPC environment.
- /// Subsequent invocations return the same instance unless Shutdown has been called first.
+ /// Returns a reference-counted instance of initialized gRPC environment.
+ /// Subsequent invocations return the same instance unless reference count has dropped to zero previously.
///
- internal static GrpcEnvironment GetInstance()
+ internal static GrpcEnvironment AddRef()
{
lock (staticLock)
{
+ refCount++;
if (instance == null)
{
instance = new GrpcEnvironment();
@@ -83,14 +85,16 @@ namespace Grpc.Core
}
///
- /// Shuts down the gRPC environment if it was initialized before.
- /// Blocks until the environment has been fully shutdown.
+ /// Decrements the reference count for currently active environment and shuts down the gRPC environment if reference count drops to zero.
+ /// (and blocks until the environment has been fully shutdown).
///
- public static void Shutdown()
+ internal static void Release()
{
lock (staticLock)
{
- if (instance != null)
+ Preconditions.CheckState(refCount > 0);
+ refCount--;
+ if (refCount == 0)
{
instance.Close();
instance = null;
@@ -125,12 +129,10 @@ namespace Grpc.Core
private GrpcEnvironment()
{
NativeLogRedirector.Redirect();
- grpcsharp_init();
+ GrpcNativeInit();
completionRegistry = new CompletionRegistry(this);
threadPool = new GrpcThreadPool(this, THREAD_POOL_SIZE);
threadPool.Start();
- // TODO: use proper logging here
- Logger.Info("gRPC initialized.");
}
///
@@ -175,6 +177,17 @@ namespace Grpc.Core
return Marshal.PtrToStringAnsi(ptr);
}
+
+ internal static void GrpcNativeInit()
+ {
+ grpcsharp_init();
+ }
+
+ internal static void GrpcNativeShutdown()
+ {
+ grpcsharp_shutdown();
+ }
+
///
/// Shuts down this environment.
///
@@ -185,12 +198,10 @@ namespace Grpc.Core
throw new InvalidOperationException("Close has already been called");
}
threadPool.Stop();
- grpcsharp_shutdown();
+ GrpcNativeShutdown();
isClosed = true;
debugStats.CheckOK();
-
- Logger.Info("gRPC shutdown.");
}
}
}
diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs
index 2c3e3d75eae..bb9ba5b8dd7 100644
--- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs
+++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs
@@ -311,9 +311,9 @@ namespace Grpc.Core.Internal
}
}
- protected override void OnReleaseResources()
+ protected override void OnAfterReleaseResources()
{
- details.Channel.Environment.DebugStats.ActiveClientCalls.Decrement();
+ details.Channel.RemoveCallReference(this);
}
private void Initialize(CompletionQueueSafeHandle cq)
@@ -323,7 +323,9 @@ namespace Grpc.Core.Internal
var call = details.Channel.Handle.CreateCall(details.Channel.Environment.CompletionRegistry,
parentCall, ContextPropagationToken.DefaultMask, cq,
details.Method, details.Host, Timespec.FromDateTime(details.Options.Deadline.Value));
- details.Channel.Environment.DebugStats.ActiveClientCalls.Increment();
+
+ details.Channel.AddCallReference(this);
+
InitializeInternal(call);
RegisterCancellationCallback();
}
diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs
index 6ca4bbdafc5..1808294f43f 100644
--- a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs
+++ b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs
@@ -189,15 +189,15 @@ namespace Grpc.Core.Internal
private void ReleaseResources()
{
- OnReleaseResources();
if (call != null)
{
call.Dispose();
}
disposed = true;
+ OnAfterReleaseResources();
}
- protected virtual void OnReleaseResources()
+ protected virtual void OnAfterReleaseResources()
{
}
@@ -212,7 +212,7 @@ namespace Grpc.Core.Internal
Preconditions.CheckState(sendCompletionDelegate == null, "Only one write can be pending at a time");
}
- protected void CheckReadingAllowed()
+ protected virtual void CheckReadingAllowed()
{
Preconditions.CheckState(started);
Preconditions.CheckState(!disposed);
diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs b/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs
index 3710a65d6bb..6278c0191ec 100644
--- a/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs
+++ b/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs
@@ -50,16 +50,19 @@ namespace Grpc.Core.Internal
readonly TaskCompletionSource