From 85030e31a545b30b84018eab5709aaba412cbbe1 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 31 May 2016 12:48:35 -0700 Subject: [PATCH 01/11] register appdomain events --- .../Grpc.Core.Tests/Grpc.Core.Tests.csproj | 1 + .../Grpc.Core.Tests/ShutdownHookTest.cs | 70 +++++++++++++++++++ src/csharp/tests.json | 1 + 3 files changed, 72 insertions(+) create mode 100644 src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index 47131fc454e..905e1e7f7e3 100644 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -86,6 +86,7 @@ + diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs new file mode 100644 index 00000000000..04649290777 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs @@ -0,0 +1,70 @@ +#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 ShutdownHookTest + { + const string Host = "127.0.0.1"; + + /// + /// Make sure that a non-shutdown channel can be cleaned up using + /// a AppDomain.ProcessExit hook. + /// + [Test] + public void AppDomainProcessExitHook() + { + var channel = new Channel(Host, 1000, ChannelCredentials.Insecure); + AppDomain.CurrentDomain.DomainUnload += (object sender, EventArgs e) => + { + Console.WriteLine("DomainUnload"); + channel.ShutdownAsync(); + }; + AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => + { + Console.WriteLine("ProcessExit"); + channel.ShutdownAsync(); + }; + } + } +} diff --git a/src/csharp/tests.json b/src/csharp/tests.json index f6af3408d51..9d37c6fc9ed 100644 --- a/src/csharp/tests.json +++ b/src/csharp/tests.json @@ -25,6 +25,7 @@ "Grpc.Core.Tests.ResponseHeadersTest", "Grpc.Core.Tests.SanityTest", "Grpc.Core.Tests.ServerTest", + "Grpc.Core.Tests.ShutdownHookTest", "Grpc.Core.Tests.ShutdownTest", "Grpc.Core.Tests.TimeoutsTest", "Grpc.Core.Tests.UserAgentStringTest" From 5858441a2cbca00b711957b3763b958ea4e43f4f Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 31 May 2016 14:32:27 -0700 Subject: [PATCH 02/11] make environment shutdown asynchronous --- .../Grpc.Core.Tests/GrpcEnvironmentTest.cs | 12 ++++++------ .../Internal/CompletionQueueSafeHandleTest.cs | 4 ++-- src/csharp/Grpc.Core.Tests/PInvokeTest.cs | 2 +- src/csharp/Grpc.Core/Channel.cs | 2 +- src/csharp/Grpc.Core/GrpcEnvironment.cs | 18 ++++++++++++------ .../Grpc.Core/Internal/GrpcThreadPool.cs | 13 +++++++++++-- src/csharp/Grpc.Core/Server.cs | 4 ++-- 7 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs b/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs index 6fe382751a1..3ec2cf48cde 100644 --- a/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs +++ b/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs @@ -49,7 +49,7 @@ namespace Grpc.Core.Tests { Assert.IsNotNull(env.CompletionQueues.ElementAt(i)); } - GrpcEnvironment.Release(); + GrpcEnvironment.ReleaseAsync().Wait(); } [Test] @@ -58,8 +58,8 @@ namespace Grpc.Core.Tests var env1 = GrpcEnvironment.AddRef(); var env2 = GrpcEnvironment.AddRef(); Assert.AreSame(env1, env2); - GrpcEnvironment.Release(); - GrpcEnvironment.Release(); + GrpcEnvironment.ReleaseAsync().Wait(); + GrpcEnvironment.ReleaseAsync().Wait(); } [Test] @@ -68,10 +68,10 @@ namespace Grpc.Core.Tests Assert.AreEqual(0, GrpcEnvironment.GetRefCount()); var env1 = GrpcEnvironment.AddRef(); - GrpcEnvironment.Release(); + GrpcEnvironment.ReleaseAsync().Wait(); var env2 = GrpcEnvironment.AddRef(); - GrpcEnvironment.Release(); + GrpcEnvironment.ReleaseAsync().Wait(); Assert.AreNotSame(env1, env2); } @@ -80,7 +80,7 @@ namespace Grpc.Core.Tests public void ReleaseWithoutAddRef() { Assert.AreEqual(0, GrpcEnvironment.GetRefCount()); - Assert.Throws(typeof(InvalidOperationException), () => GrpcEnvironment.Release()); + Assert.ThrowsAsync(typeof(InvalidOperationException), async () => await GrpcEnvironment.ReleaseAsync()); } [Test] diff --git a/src/csharp/Grpc.Core.Tests/Internal/CompletionQueueSafeHandleTest.cs b/src/csharp/Grpc.Core.Tests/Internal/CompletionQueueSafeHandleTest.cs index 195119f920d..e9ec59eb3db 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/CompletionQueueSafeHandleTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/CompletionQueueSafeHandleTest.cs @@ -48,7 +48,7 @@ namespace Grpc.Core.Internal.Tests GrpcEnvironment.AddRef(); var cq = CompletionQueueSafeHandle.Create(); cq.Dispose(); - GrpcEnvironment.Release(); + GrpcEnvironment.ReleaseAsync().Wait(); } [Test] @@ -59,7 +59,7 @@ namespace Grpc.Core.Internal.Tests cq.Shutdown(); var ev = cq.Next(); cq.Dispose(); - GrpcEnvironment.Release(); + GrpcEnvironment.ReleaseAsync().Wait(); Assert.AreEqual(CompletionQueueEvent.CompletionType.Shutdown, ev.type); Assert.AreNotEqual(IntPtr.Zero, ev.success); Assert.AreEqual(IntPtr.Zero, ev.tag); diff --git a/src/csharp/Grpc.Core.Tests/PInvokeTest.cs b/src/csharp/Grpc.Core.Tests/PInvokeTest.cs index d2b2fc6a661..d3735c78807 100644 --- a/src/csharp/Grpc.Core.Tests/PInvokeTest.cs +++ b/src/csharp/Grpc.Core.Tests/PInvokeTest.cs @@ -65,7 +65,7 @@ namespace Grpc.Core.Tests cq.Dispose(); }); - GrpcEnvironment.Release(); + GrpcEnvironment.ReleaseAsync().Wait(); } /// diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs index 9cee7526630..b58a6a7381b 100644 --- a/src/csharp/Grpc.Core/Channel.cs +++ b/src/csharp/Grpc.Core/Channel.cs @@ -220,7 +220,7 @@ namespace Grpc.Core handle.Dispose(); - await Task.Run(() => GrpcEnvironment.Release()).ConfigureAwait(false); + await GrpcEnvironment.ReleaseAsync().ConfigureAwait(false); } internal ChannelSafeHandle Handle diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs index 18af1099f1b..6e56b6e8e3c 100644 --- a/src/csharp/Grpc.Core/GrpcEnvironment.cs +++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs @@ -35,6 +35,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Runtime.InteropServices; +using System.Threading.Tasks; using Grpc.Core.Internal; using Grpc.Core.Logging; using Grpc.Core.Utils; @@ -79,21 +80,26 @@ namespace Grpc.Core } /// - /// 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). + /// Decrements the reference count for currently active environment and asynchronously shuts down the gRPC environment if reference count drops to zero. /// - internal static void Release() + internal static async Task ReleaseAsync() { + GrpcEnvironment instanceToShutdown = null; lock (staticLock) { GrpcPreconditions.CheckState(refCount > 0); refCount--; if (refCount == 0) { - instance.Close(); + instanceToShutdown = instance; instance = null; } } + + if (instanceToShutdown != null) + { + await instanceToShutdown.ShutdownAsync(); + } } internal static int GetRefCount() @@ -223,13 +229,13 @@ namespace Grpc.Core /// /// Shuts down this environment. /// - private void Close() + private async Task ShutdownAsync() { if (isClosed) { throw new InvalidOperationException("Close has already been called"); } - threadPool.Stop(); + await threadPool.StopAsync().ConfigureAwait(false); GrpcNativeShutdown(); isClosed = true; diff --git a/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs b/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs index 4de543bef7a..f50f2a6e39f 100644 --- a/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs +++ b/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs @@ -35,6 +35,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading; +using System.Threading.Tasks; using Grpc.Core.Logging; using Grpc.Core.Utils; @@ -53,6 +54,8 @@ namespace Grpc.Core.Internal readonly int poolSize; readonly int completionQueueCount; + bool stopRequested; + IReadOnlyCollection completionQueues; /// @@ -84,15 +87,21 @@ namespace Grpc.Core.Internal } } - public void Stop() + public Task StopAsync() { lock (myLock) { + GrpcPreconditions.CheckState(!stopRequested, "Stop already requested."); + stopRequested = true; + foreach (var cq in completionQueues) { cq.Shutdown(); } + } + return Task.Run(() => + { foreach (var thread in threads) { thread.Join(); @@ -102,7 +111,7 @@ namespace Grpc.Core.Internal { cq.Dispose(); } - } + }); } internal IReadOnlyCollection CompletionQueues diff --git a/src/csharp/Grpc.Core/Server.cs b/src/csharp/Grpc.Core/Server.cs index 6bd79005617..18a808e604e 100644 --- a/src/csharp/Grpc.Core/Server.cs +++ b/src/csharp/Grpc.Core/Server.cs @@ -169,7 +169,7 @@ namespace Grpc.Core await shutdownTcs.Task.ConfigureAwait(false); DisposeHandle(); - await Task.Run(() => GrpcEnvironment.Release()).ConfigureAwait(false); + await GrpcEnvironment.ReleaseAsync().ConfigureAwait(false); } /// @@ -194,7 +194,7 @@ namespace Grpc.Core await shutdownTcs.Task.ConfigureAwait(false); DisposeHandle(); - await Task.Run(() => GrpcEnvironment.Release()).ConfigureAwait(false); + await GrpcEnvironment.ReleaseAsync().ConfigureAwait(false); } internal void AddCallReference(object call) From 703c042ec00f1bef0b7e1e62276fe8b7fb36391a Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 31 May 2016 15:46:10 -0700 Subject: [PATCH 03/11] serverside shutdown hook test --- .../Grpc.Core.Tests/Grpc.Core.Tests.csproj | 1 + .../Grpc.Core.Tests/ShutdownHookServerTest.cs | 75 +++++++++++++++++++ .../Grpc.Core.Tests/ShutdownHookTest.cs | 13 ++-- .../Grpc.Core/Internal/GrpcThreadPool.cs | 2 +- src/csharp/tests.json | 1 + 5 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index 905e1e7f7e3..0a1e8a69380 100644 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -87,6 +87,7 @@ + diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs new file mode 100644 index 00000000000..7990a7d15f1 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs @@ -0,0 +1,75 @@ +#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 ShutdownHookServerTest + { + const string Host = "127.0.0.1"; + + /// + /// Make sure that a non-shutdown server can be cleaned up using + /// a AppDomain.ProcessExit hook. + /// + [Test] + public void AppDomainProcessExitHook() + { + var helper = new MockServiceHelper(Host); + var server = helper.GetServer(); + server.Start(); + AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => + { + // TODO: expose API for killing all servers + // TODO: expose API for closing all channels + server.KillAsync(); + GrpcEnvironment.ReleaseAsync(); + }; + } + + // TODO: test what happens if there's a pending completion in the cq (destroy on non-empty cq) + + // TODO: test what happens if there's an appdomain unload + + // TODO: tests involving a server... + } +} diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs index 04649290777..301ae091c61 100644 --- a/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs @@ -55,16 +55,17 @@ namespace Grpc.Core.Tests public void AppDomainProcessExitHook() { var channel = new Channel(Host, 1000, ChannelCredentials.Insecure); - AppDomain.CurrentDomain.DomainUnload += (object sender, EventArgs e) => - { - Console.WriteLine("DomainUnload"); - channel.ShutdownAsync(); - }; AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => { - Console.WriteLine("ProcessExit"); + // TODO: expose API to shutdown all channels. channel.ShutdownAsync(); }; } + + // TODO: test what happens if there's a pending completion in the cq (destroy on non-empty cq) + + // TODO: test what happens if there's an appdomain unload + + // TODO: tests involving a server... } } diff --git a/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs b/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs index f50f2a6e39f..8643abf5364 100644 --- a/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs +++ b/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs @@ -128,7 +128,7 @@ namespace Grpc.Core.Internal var cq = completionQueues.ElementAt(cqIndex); var thread = new Thread(new ThreadStart(() => RunHandlerLoop(cq))); - thread.IsBackground = false; + thread.IsBackground = true; thread.Name = string.Format("grpc {0} (cq {1})", threadIndex, cqIndex); thread.Start(); diff --git a/src/csharp/tests.json b/src/csharp/tests.json index 9d37c6fc9ed..d80e95a3a4d 100644 --- a/src/csharp/tests.json +++ b/src/csharp/tests.json @@ -25,6 +25,7 @@ "Grpc.Core.Tests.ResponseHeadersTest", "Grpc.Core.Tests.SanityTest", "Grpc.Core.Tests.ServerTest", + "Grpc.Core.Tests.ShutdownHookServerTest", "Grpc.Core.Tests.ShutdownHookTest", "Grpc.Core.Tests.ShutdownTest", "Grpc.Core.Tests.TimeoutsTest", From 4aea5281de1fb5686ae5bb6305e51b704ac57317 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 1 Jun 2016 12:42:54 -0700 Subject: [PATCH 04/11] Add ShutdownChannelsAsync api --- src/csharp/Grpc.Core/Channel.cs | 2 ++ src/csharp/Grpc.Core/GrpcEnvironment.cs | 34 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs index b58a6a7381b..e0fc7180daf 100644 --- a/src/csharp/Grpc.Core/Channel.cs +++ b/src/csharp/Grpc.Core/Channel.cs @@ -88,6 +88,7 @@ namespace Grpc.Core this.handle = ChannelSafeHandle.CreateInsecure(target, nativeChannelArgs); } } + GrpcEnvironment.RegisterChannel(this); } /// @@ -209,6 +210,7 @@ namespace Grpc.Core GrpcPreconditions.CheckState(!shutdownRequested); shutdownRequested = true; } + GrpcEnvironment.UnregisterChannel(this); shutdownTokenSource.Cancel(); diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs index 6e56b6e8e3c..c25022a5d4b 100644 --- a/src/csharp/Grpc.Core/GrpcEnvironment.cs +++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs @@ -54,12 +54,15 @@ namespace Grpc.Core static int refCount; static int? customThreadPoolSize; static int? customCompletionQueueCount; + static readonly HashSet registeredChannels = new HashSet(); static ILogger logger = new ConsoleLogger(); + readonly object myLock = new object(); readonly GrpcThreadPool threadPool; readonly DebugStats debugStats = new DebugStats(); readonly AtomicCounter cqPickerCounter = new AtomicCounter(); + bool isClosed; /// @@ -110,6 +113,37 @@ namespace Grpc.Core } } + internal static void RegisterChannel(Channel channel) + { + lock (staticLock) + { + GrpcPreconditions.CheckNotNull(channel); + registeredChannels.Add(channel); + } + } + + internal static void UnregisterChannel(Channel channel) + { + lock (staticLock) + { + GrpcPreconditions.CheckNotNull(channel); + GrpcPreconditions.CheckArgument(registeredChannels.Remove(channel), "Channel not found in the registered channels set."); + } + } + + /// + /// Requests shutdown of all channels created by the current process. + /// + public static Task ShutdownChannelsAsync() + { + HashSet snapshot = null; + lock (staticLock) + { + snapshot = new HashSet(registeredChannels); + } + return Task.WhenAll(snapshot.Select((channel) => channel.ShutdownAsync())); + } + /// /// Gets application-wide logger used by gRPC. /// From 63386a1064f9b27b0590c1e10f7176a45b0a3f36 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 1 Jun 2016 12:47:46 -0700 Subject: [PATCH 05/11] deduplicate server shutdown logic --- src/csharp/Grpc.Core/Server.cs | 57 +++++++++++++++++----------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/csharp/Grpc.Core/Server.cs b/src/csharp/Grpc.Core/Server.cs index 18a808e604e..88045a51c85 100644 --- a/src/csharp/Grpc.Core/Server.cs +++ b/src/csharp/Grpc.Core/Server.cs @@ -155,21 +155,9 @@ namespace Grpc.Core /// /// It is strongly recommended to shutdown all previously created servers before exiting from the process. /// - public async Task ShutdownAsync() + public Task ShutdownAsync() { - lock (myLock) - { - GrpcPreconditions.CheckState(startRequested); - GrpcPreconditions.CheckState(!shutdownRequested); - shutdownRequested = true; - } - - var cq = environment.CompletionQueues.First(); // any cq will do - handle.ShutdownAndNotify(HandleServerShutdown, cq); - await shutdownTcs.Task.ConfigureAwait(false); - DisposeHandle(); - - await GrpcEnvironment.ReleaseAsync().ConfigureAwait(false); + return ShutdownInternalAsync(false); } /// @@ -179,22 +167,9 @@ namespace Grpc.Core /// /// It is strongly recommended to shutdown all previously created servers before exiting from the process. /// - public async Task KillAsync() + public Task KillAsync() { - lock (myLock) - { - GrpcPreconditions.CheckState(startRequested); - GrpcPreconditions.CheckState(!shutdownRequested); - shutdownRequested = true; - } - - var cq = environment.CompletionQueues.First(); // any cq will do - handle.ShutdownAndNotify(HandleServerShutdown, cq); - handle.CancelAllCalls(); - await shutdownTcs.Task.ConfigureAwait(false); - DisposeHandle(); - - await GrpcEnvironment.ReleaseAsync().ConfigureAwait(false); + return ShutdownInternalAsync(true); } internal void AddCallReference(object call) @@ -212,6 +187,30 @@ namespace Grpc.Core activeCallCounter.Decrement(); } + /// + /// Shuts down the server. + /// + private async Task ShutdownInternalAsync(bool kill) + { + lock (myLock) + { + GrpcPreconditions.CheckState(startRequested); + GrpcPreconditions.CheckState(!shutdownRequested); + shutdownRequested = true; + } + + var cq = environment.CompletionQueues.First(); // any cq will do + handle.ShutdownAndNotify(HandleServerShutdown, cq); + if (kill) + { + handle.CancelAllCalls(); + } + await shutdownTcs.Task.ConfigureAwait(false); + DisposeHandle(); + + await GrpcEnvironment.ReleaseAsync().ConfigureAwait(false); + } + /// /// Adds a service definition. /// From 739ee1b159cd0925cbc448c4b95728926f1a0e60 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 1 Jun 2016 14:08:26 -0700 Subject: [PATCH 06/11] support GrpcEnvironment.KillServersAsync --- src/csharp/Grpc.Core/GrpcEnvironment.cs | 40 +++++++++++++++++++ .../Grpc.Core/Internal/GrpcThreadPool.cs | 15 +++++++ src/csharp/Grpc.Core/Server.cs | 26 +++++++++++- 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs index c25022a5d4b..ceaa2ec439a 100644 --- a/src/csharp/Grpc.Core/GrpcEnvironment.cs +++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs @@ -55,6 +55,7 @@ namespace Grpc.Core static int? customThreadPoolSize; static int? customCompletionQueueCount; static readonly HashSet registeredChannels = new HashSet(); + static readonly HashSet registeredServers = new HashSet(); static ILogger logger = new ConsoleLogger(); @@ -131,6 +132,24 @@ namespace Grpc.Core } } + internal static void RegisterServer(Server server) + { + lock (staticLock) + { + GrpcPreconditions.CheckNotNull(server); + registeredServers.Add(server); + } + } + + internal static void UnregisterServer(Server server) + { + lock (staticLock) + { + GrpcPreconditions.CheckNotNull(server); + GrpcPreconditions.CheckArgument(registeredServers.Remove(server), "Server not found in the registered servers set."); + } + } + /// /// Requests shutdown of all channels created by the current process. /// @@ -144,6 +163,19 @@ namespace Grpc.Core return Task.WhenAll(snapshot.Select((channel) => channel.ShutdownAsync())); } + /// + /// Requests immediate shutdown of all servers created by the current process. + /// + public static Task KillServersAsync() + { + HashSet snapshot = null; + lock (staticLock) + { + snapshot = new HashSet(registeredServers); + } + return Task.WhenAll(snapshot.Select((server) => server.KillAsync())); + } + /// /// Gets application-wide logger used by gRPC. /// @@ -220,6 +252,14 @@ namespace Grpc.Core } } + internal bool IsAlive + { + get + { + return this.threadPool.IsAlive; + } + } + /// /// Picks a completion queue in a round-robin fashion. /// Shouldn't be invoked on a per-call basis (used at per-channel basis). diff --git a/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs b/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs index 8643abf5364..a446c1f99f2 100644 --- a/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs +++ b/src/csharp/Grpc.Core/Internal/GrpcThreadPool.cs @@ -114,6 +114,21 @@ namespace Grpc.Core.Internal }); } + /// + /// Returns true if there is at least one thread pool thread that hasn't + /// already stopped. + /// Threads can either stop because all completion queues shut down or + /// because all foreground threads have already shutdown and process is + /// going to exit. + /// + internal bool IsAlive + { + get + { + return threads.Any(t => t.ThreadState != ThreadState.Stopped); + } + } + internal IReadOnlyCollection CompletionQueues { get diff --git a/src/csharp/Grpc.Core/Server.cs b/src/csharp/Grpc.Core/Server.cs index 88045a51c85..e3468ee8420 100644 --- a/src/csharp/Grpc.Core/Server.cs +++ b/src/csharp/Grpc.Core/Server.cs @@ -86,6 +86,7 @@ namespace Grpc.Core { this.handle.RegisterCompletionQueue(cq); } + GrpcEnvironment.RegisterServer(this); } /// @@ -198,6 +199,7 @@ namespace Grpc.Core GrpcPreconditions.CheckState(!shutdownRequested); shutdownRequested = true; } + GrpcEnvironment.UnregisterServer(this); var cq = environment.CompletionQueues.First(); // any cq will do handle.ShutdownAndNotify(HandleServerShutdown, cq); @@ -205,12 +207,34 @@ namespace Grpc.Core { handle.CancelAllCalls(); } - await shutdownTcs.Task.ConfigureAwait(false); + + await ShutdownCompleteOrEnvironmentDeadAsync().ConfigureAwait(false); + DisposeHandle(); await GrpcEnvironment.ReleaseAsync().ConfigureAwait(false); } + /// + /// In case the environment's threadpool becomes dead, the shutdown completion will + /// never be delivered, but we need to release the environment's handle anyway. + /// + private async Task ShutdownCompleteOrEnvironmentDeadAsync() + { + while (true) + { + var task = await Task.WhenAny(shutdownTcs.Task, Task.Delay(20)).ConfigureAwait(false); + if (shutdownTcs.Task == task) + { + return; + } + if (!environment.IsAlive) + { + return; + } + } + } + /// /// Adds a service definition. /// From 25e3ba57b135d75ae1985fc1a94b5c498b8c8a40 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 1 Jun 2016 14:10:29 -0700 Subject: [PATCH 07/11] improve shutdown tests --- src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs | 7 +++---- src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs index 7990a7d15f1..2f40e1a8ef2 100644 --- a/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs @@ -59,10 +59,9 @@ namespace Grpc.Core.Tests server.Start(); AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => { - // TODO: expose API for killing all servers - // TODO: expose API for closing all channels - server.KillAsync(); - GrpcEnvironment.ReleaseAsync(); + var shutdownChannelsTask = GrpcEnvironment.ShutdownChannelsAsync(); + var killServersTask = GrpcEnvironment.KillServersAsync(); + Task.WaitAll(shutdownChannelsTask, killServersTask); }; } diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs index 301ae091c61..da71ed7e2ec 100644 --- a/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs @@ -55,10 +55,10 @@ namespace Grpc.Core.Tests public void AppDomainProcessExitHook() { var channel = new Channel(Host, 1000, ChannelCredentials.Insecure); + var channel2 = new Channel(Host, 1001, ChannelCredentials.Insecure); AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => { - // TODO: expose API to shutdown all channels. - channel.ShutdownAsync(); + GrpcEnvironment.ShutdownChannelsAsync().Wait(); }; } From 018cfb8c14954b8c8b0d137dcd23d5bfc6919738 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 1 Jun 2016 14:44:09 -0700 Subject: [PATCH 08/11] update tests --- .../Grpc.Core.Tests/Grpc.Core.Tests.csproj | 3 +- ...nHookTest.cs => ShutdownHookClientTest.cs} | 13 +--- .../ShutdownHookPendingCallTest.cs | 76 +++++++++++++++++++ .../Grpc.Core.Tests/ShutdownHookServerTest.cs | 12 +-- src/csharp/tests.json | 3 +- 5 files changed, 83 insertions(+), 24 deletions(-) rename src/csharp/Grpc.Core.Tests/{ShutdownHookTest.cs => ShutdownHookClientTest.cs} (85%) create mode 100644 src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index 0a1e8a69380..d6adb5bb81c 100644 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -86,8 +86,9 @@ - + + diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookClientTest.cs similarity index 85% rename from src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs rename to src/csharp/Grpc.Core.Tests/ShutdownHookClientTest.cs index da71ed7e2ec..32c9d8034a5 100644 --- a/src/csharp/Grpc.Core.Tests/ShutdownHookTest.cs +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookClientTest.cs @@ -43,16 +43,12 @@ using NUnit.Framework; namespace Grpc.Core.Tests { - public class ShutdownHookTest + public class ShutdownHookClientTest { const string Host = "127.0.0.1"; - /// - /// Make sure that a non-shutdown channel can be cleaned up using - /// a AppDomain.ProcessExit hook. - /// [Test] - public void AppDomainProcessExitHook() + public void ProcessExitHookCanCleanupAbandonedChannels() { var channel = new Channel(Host, 1000, ChannelCredentials.Insecure); var channel2 = new Channel(Host, 1001, ChannelCredentials.Insecure); @@ -61,11 +57,6 @@ namespace Grpc.Core.Tests GrpcEnvironment.ShutdownChannelsAsync().Wait(); }; } - - // TODO: test what happens if there's a pending completion in the cq (destroy on non-empty cq) - // TODO: test what happens if there's an appdomain unload - - // TODO: tests involving a server... } } diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs new file mode 100644 index 00000000000..7f3f493a481 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs @@ -0,0 +1,76 @@ +#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 ShutdownHookPendingCallTest + { + const string Host = "127.0.0.1"; + + [Test] + public void ProcessExitHookCanCleanupAbandonedCall() + { + AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => + { + var shutdownChannelsTask = GrpcEnvironment.ShutdownChannelsAsync(); + var killServersTask = GrpcEnvironment.KillServersAsync(); + Task.WaitAll(shutdownChannelsTask, killServersTask); + }; + + var helper = new MockServiceHelper(Host); + var server = helper.GetServer(); + server.Start(); + var channel = helper.GetChannel(); + + var readyToShutdown = new TaskCompletionSource(); + helper.DuplexStreamingHandler = new DuplexStreamingServerMethod(async (requestStream, responseStream, context) => + { + readyToShutdown.SetResult(null); + await requestStream.ToListAsync(); + }); + + var call = Calls.AsyncDuplexStreamingCall(helper.CreateDuplexStreamingCall()); + readyToShutdown.Task.Wait(); // make sure handler is running + } + } +} diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs index 2f40e1a8ef2..b223f5ee02c 100644 --- a/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs @@ -47,12 +47,8 @@ namespace Grpc.Core.Tests { const string Host = "127.0.0.1"; - /// - /// Make sure that a non-shutdown server can be cleaned up using - /// a AppDomain.ProcessExit hook. - /// [Test] - public void AppDomainProcessExitHook() + public void ProcessExitHookCanCleanupAbandonedServers() { var helper = new MockServiceHelper(Host); var server = helper.GetServer(); @@ -64,11 +60,5 @@ namespace Grpc.Core.Tests Task.WaitAll(shutdownChannelsTask, killServersTask); }; } - - // TODO: test what happens if there's a pending completion in the cq (destroy on non-empty cq) - - // TODO: test what happens if there's an appdomain unload - - // TODO: tests involving a server... } } diff --git a/src/csharp/tests.json b/src/csharp/tests.json index d80e95a3a4d..7a106aa0f92 100644 --- a/src/csharp/tests.json +++ b/src/csharp/tests.json @@ -25,8 +25,9 @@ "Grpc.Core.Tests.ResponseHeadersTest", "Grpc.Core.Tests.SanityTest", "Grpc.Core.Tests.ServerTest", + "Grpc.Core.Tests.ShutdownHookClientTest", + "Grpc.Core.Tests.ShutdownHookPendingCallTest", "Grpc.Core.Tests.ShutdownHookServerTest", - "Grpc.Core.Tests.ShutdownHookTest", "Grpc.Core.Tests.ShutdownTest", "Grpc.Core.Tests.TimeoutsTest", "Grpc.Core.Tests.UserAgentStringTest" From 2f7414117cadca069b85789011ea43a6e262092d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 1 Jun 2016 16:07:15 -0700 Subject: [PATCH 09/11] add appdomain unload test --- .../Grpc.Core.Tests/AppDomainUnloadTest.cs | 97 +++++++++++++++++++ .../Grpc.Core.Tests/Grpc.Core.Tests.csproj | 1 + src/csharp/tests.json | 1 + 3 files changed, 99 insertions(+) create mode 100644 src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs diff --git a/src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs b/src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs new file mode 100644 index 00000000000..60aae0e1e57 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs @@ -0,0 +1,97 @@ +#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.Reflection; +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 AppDomainUnloadTest + { + [Test] + public void AppDomainUnloadHookCanCleanupAbandonedCall() + { + var setup = new AppDomainSetup + { + ApplicationBase = AppDomain.CurrentDomain.BaseDirectory + }; + var childDomain = AppDomain.CreateDomain("test", null, setup); + var remoteObj = childDomain.CreateInstance(typeof(AppDomainTestClass).Assembly.GetName().Name, typeof(AppDomainTestClass).FullName); + + // Try to unload the appdomain once we've created a server and a channel inside the appdomain. + AppDomain.Unload(childDomain); + } + + public class AppDomainTestClass + { + const string Host = "127.0.0.1"; + + /// + /// Creates a server and a channel and initiates a call. The code is invoked from inside of an AppDomain + /// to test if AppDomain.Unload() work if Grpc is being used. + /// + public AppDomainTestClass() + { + AppDomain.CurrentDomain.DomainUnload += (object sender, EventArgs e) => + { + var shutdownChannelsTask = GrpcEnvironment.ShutdownChannelsAsync(); + var killServersTask = GrpcEnvironment.KillServersAsync(); + Task.WaitAll(shutdownChannelsTask, killServersTask); + }; + + var helper = new MockServiceHelper(Host); + var server = helper.GetServer(); + server.Start(); + var channel = helper.GetChannel(); + + var readyToShutdown = new TaskCompletionSource(); + helper.DuplexStreamingHandler = new DuplexStreamingServerMethod(async (requestStream, responseStream, context) => + { + readyToShutdown.SetResult(null); + await requestStream.ToListAsync(); + }); + + var call = Calls.AsyncDuplexStreamingCall(helper.CreateDuplexStreamingCall()); + readyToShutdown.Task.Wait(); // make sure handler is running + } + } + } +} diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index d6adb5bb81c..074c9603dcf 100644 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -89,6 +89,7 @@ + diff --git a/src/csharp/tests.json b/src/csharp/tests.json index 7a106aa0f92..7e7aee1093a 100644 --- a/src/csharp/tests.json +++ b/src/csharp/tests.json @@ -7,6 +7,7 @@ "Grpc.Core.Internal.Tests.CompletionQueueSafeHandleTest", "Grpc.Core.Internal.Tests.MetadataArraySafeHandleTest", "Grpc.Core.Internal.Tests.TimespecTest", + "Grpc.Core.Tests.AppDomainUnloadTest", "Grpc.Core.Tests.CallCredentialsTest", "Grpc.Core.Tests.CallOptionsTest", "Grpc.Core.Tests.ChannelCredentialsTest", From bdccdef0c6f0485f79d68985a46be878c5d26ce8 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 1 Jun 2016 16:29:14 -0700 Subject: [PATCH 10/11] autoregister grpc shutdown hooks --- src/csharp/Grpc.Core/GrpcEnvironment.cs | 29 +++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs index ceaa2ec439a..0359d9092a7 100644 --- a/src/csharp/Grpc.Core/GrpcEnvironment.cs +++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs @@ -72,6 +72,8 @@ namespace Grpc.Core /// internal static GrpcEnvironment AddRef() { + ShutdownHooks.Register(); + lock (staticLock) { refCount++; @@ -337,5 +339,32 @@ namespace Grpc.Core // by default, create a completion queue for each thread return GetThreadPoolSizeOrDefault(); } + + private static class ShutdownHooks + { + static object staticLock = new object(); + static bool hooksRegistered; + + public static void Register() + { + lock (staticLock) + { + if (!hooksRegistered) + { + AppDomain.CurrentDomain.ProcessExit += ShutdownHookHandler; + AppDomain.CurrentDomain.DomainUnload += ShutdownHookHandler; + } + hooksRegistered = true; + } + } + + /// + /// Handler for AppDomain.DomainUnload and AppDomain.ProcessExit hooks. + /// + private static void ShutdownHookHandler(object sender, EventArgs e) + { + Task.WaitAll(GrpcEnvironment.ShutdownChannelsAsync(), GrpcEnvironment.KillServersAsync()); + } + } } } From ed5af1c6232ecf67b624d2a75b3960b838801698 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 1 Jun 2016 16:29:37 -0700 Subject: [PATCH 11/11] update tests --- src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs | 7 ------- src/csharp/Grpc.Core.Tests/ShutdownHookClientTest.cs | 5 ----- src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs | 7 ------- src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs | 6 ------ 4 files changed, 25 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs b/src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs index 60aae0e1e57..e605a310f9e 100644 --- a/src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs +++ b/src/csharp/Grpc.Core.Tests/AppDomainUnloadTest.cs @@ -70,13 +70,6 @@ namespace Grpc.Core.Tests /// public AppDomainTestClass() { - AppDomain.CurrentDomain.DomainUnload += (object sender, EventArgs e) => - { - var shutdownChannelsTask = GrpcEnvironment.ShutdownChannelsAsync(); - var killServersTask = GrpcEnvironment.KillServersAsync(); - Task.WaitAll(shutdownChannelsTask, killServersTask); - }; - var helper = new MockServiceHelper(Host); var server = helper.GetServer(); server.Start(); diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookClientTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookClientTest.cs index 32c9d8034a5..12b8452f64f 100644 --- a/src/csharp/Grpc.Core.Tests/ShutdownHookClientTest.cs +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookClientTest.cs @@ -52,11 +52,6 @@ namespace Grpc.Core.Tests { var channel = new Channel(Host, 1000, ChannelCredentials.Insecure); var channel2 = new Channel(Host, 1001, ChannelCredentials.Insecure); - AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => - { - GrpcEnvironment.ShutdownChannelsAsync().Wait(); - }; } - // TODO: test what happens if there's an appdomain unload } } diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs index 7f3f493a481..175233840de 100644 --- a/src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookPendingCallTest.cs @@ -50,13 +50,6 @@ namespace Grpc.Core.Tests [Test] public void ProcessExitHookCanCleanupAbandonedCall() { - AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => - { - var shutdownChannelsTask = GrpcEnvironment.ShutdownChannelsAsync(); - var killServersTask = GrpcEnvironment.KillServersAsync(); - Task.WaitAll(shutdownChannelsTask, killServersTask); - }; - var helper = new MockServiceHelper(Host); var server = helper.GetServer(); server.Start(); diff --git a/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs b/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs index b223f5ee02c..e7ea7a0bf59 100644 --- a/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ShutdownHookServerTest.cs @@ -53,12 +53,6 @@ namespace Grpc.Core.Tests var helper = new MockServiceHelper(Host); var server = helper.GetServer(); server.Start(); - AppDomain.CurrentDomain.ProcessExit += (object sender, EventArgs e) => - { - var shutdownChannelsTask = GrpcEnvironment.ShutdownChannelsAsync(); - var killServersTask = GrpcEnvironment.KillServersAsync(); - Task.WaitAll(shutdownChannelsTask, killServersTask); - }; } } }