From cd01eef931560789b0e16233cc1c5d612efc4496 Mon Sep 17 00:00:00 2001 From: Chris Bacon Date: Mon, 9 Jul 2018 13:35:00 +0100 Subject: [PATCH 1/3] Remove C# shutdown hook on .NET Core --- src/csharp/Grpc.Core/GrpcEnvironment.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs index 6bb2f6c3e54..b50dfedc573 100644 --- a/src/csharp/Grpc.Core/GrpcEnvironment.cs +++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs @@ -423,7 +423,7 @@ namespace Grpc.Core if (!hooksRegistered) { #if NETSTANDARD1_5 - System.Runtime.Loader.AssemblyLoadContext.Default.Unloading += (assemblyLoadContext) => { HandleShutdown(); }; + // No action required at shutdown on .NET Core #else AppDomain.CurrentDomain.ProcessExit += (sender, eventArgs) => { HandleShutdown(); }; AppDomain.CurrentDomain.DomainUnload += (sender, eventArgs) => { HandleShutdown(); }; From df332f36ff5ab32a9d8eea8f883cb768d317244c Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 10 Jul 2018 17:28:14 +0200 Subject: [PATCH 2/3] explain the shutdown hooks in detail --- src/csharp/Grpc.Core/GrpcEnvironment.cs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs index b50dfedc573..0f7474cdae5 100644 --- a/src/csharp/Grpc.Core/GrpcEnvironment.cs +++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs @@ -422,9 +422,30 @@ namespace Grpc.Core { if (!hooksRegistered) { + // Under normal circumstances, the user is expected to shutdown all + // the gRPC channels and servers before the application exits. The following + // hooks provide some extra handling for cases when this is not the case, + // in the effort to achieve a reasonable behavior on shutdown. #if NETSTANDARD1_5 // No action required at shutdown on .NET Core + // - In-progress P/Invoke calls (such as grpc_completion_queue_next) don't seem + // to prevent a .NET core application from terminating, so no special handling + // is needed. + // - .NET core doesn't run finalizers on shutdown, so there's no risk of getting + // a crash because grpc_*_destroy methods for native objects being invoked + // in wrong order. #else + // On desktop .NET framework and Mono, we need to register for a shutdown + // event to explicitly shutdown the GrpcEnvironment. + // - On Desktop .NET framework, we need to do a proper shutdown to prevent a crash + // when the framework attempts to run the finalizers for SafeHandle object representing the native + // grpc objects. The finalizers calls the native grpc_*_destroy methods (e.g. grpc_server_destroy) + // in a random order, which is not supported by gRPC. + // - On Mono, the process would hang as the GrpcThreadPool threads are sleeping + // in grpc_completion_queue_next P/Invoke invocation and mono won't let the + // process shutdown until the P/Invoke calls return. We achieve that by shutting down + // the completion queue(s) which associated with the GrpcThreadPool, which will + // cause the grpc_completion_queue_next calls to return immediately. AppDomain.CurrentDomain.ProcessExit += (sender, eventArgs) => { HandleShutdown(); }; AppDomain.CurrentDomain.DomainUnload += (sender, eventArgs) => { HandleShutdown(); }; #endif From f5b19665ccd1ee32394ca5c3a6aa9dd07e6f4023 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 10 Jul 2018 17:29:58 +0200 Subject: [PATCH 3/3] add a TODO --- src/csharp/Grpc.Core/GrpcEnvironment.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs index 0f7474cdae5..a6a1d8af50e 100644 --- a/src/csharp/Grpc.Core/GrpcEnvironment.cs +++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs @@ -434,6 +434,8 @@ namespace Grpc.Core // - .NET core doesn't run finalizers on shutdown, so there's no risk of getting // a crash because grpc_*_destroy methods for native objects being invoked // in wrong order. + // TODO(jtattermusch): Verify that the shutdown hooks are still not needed + // once we add support for new platforms using netstandard (e.g. Xamarin). #else // On desktop .NET framework and Mono, we need to register for a shutdown // event to explicitly shutdown the GrpcEnvironment.