From 34d131e70ff1eb7ea049386ebb45e28c13f8656b Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 19 May 2020 13:58:36 +0200 Subject: [PATCH] add a repro and fix for #19090 --- .../Grpc.Core.Tests/CallAfterShutdownTest.cs | 48 +++++++++++++++++++ src/csharp/Grpc.Core/Internal/AsyncCall.cs | 7 ++- src/csharp/tests.json | 1 + 3 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 src/csharp/Grpc.Core.Tests/CallAfterShutdownTest.cs diff --git a/src/csharp/Grpc.Core.Tests/CallAfterShutdownTest.cs b/src/csharp/Grpc.Core.Tests/CallAfterShutdownTest.cs new file mode 100644 index 0000000000000..c1cf7e68ad04d --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/CallAfterShutdownTest.cs @@ -0,0 +1,48 @@ +#region Copyright notice and license + +// Copyright 2020 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using System; +using NUnit.Framework; + +namespace Grpc.Core.Tests +{ + public class CallAfterShutdownTest + { + Method dummyUnaryMethod = new Method(MethodType.Unary, "fooservice", "dummyMethod", Marshallers.StringMarshaller, Marshallers.StringMarshaller); + + [Test] + public void StartBlockingUnaryCallAfterChannelShutdown() + { + // create a channel and immediately shut it down. + var channel = new Channel("127.0.0.1", 1000, ChannelCredentials.Insecure); + channel.ShutdownAsync().Wait(); // also shuts down GrpcEnvironment + + Assert.Throws(typeof(ObjectDisposedException), () => Calls.BlockingUnaryCall(new CallInvocationDetails(channel, dummyUnaryMethod, new CallOptions()), "THE REQUEST")); + } + + [Test] + public void StartAsyncUnaryCallAfterChannelShutdown() + { + // create a channel and immediately shut it down. + var channel = new Channel("127.0.0.1", 1000, ChannelCredentials.Insecure); + channel.ShutdownAsync().Wait(); // also shuts down GrpcEnvironment + + Assert.Throws(typeof(ObjectDisposedException), () => Calls.AsyncUnaryCall(new CallInvocationDetails(channel, dummyUnaryMethod, new CallOptions()), "THE REQUEST")); + } + } +} diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs index 500842f7c6361..1e570b091de9c 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs @@ -78,7 +78,12 @@ public TResponse UnaryCall(TRequest msg) var profiler = Profilers.ForCurrentThread(); using (profiler.NewScope("AsyncCall.UnaryCall")) - using (CompletionQueueSafeHandle cq = CompletionQueueSafeHandle.CreateSync()) + + // Create a pluckable completion queue for the call. Avoid creating a completion queue when we know the channel has already + // been shutdown. In such case, the call will fail with ObjectDisposedException immediately anyway and creating / destroying + // a completion queue would lead to crash if this was the last channel in the application (and thus GrpcEnvironment has been shutdown). + // See https://github.com/grpc/grpc/issues/19090 + using (CompletionQueueSafeHandle cq = details.Channel.Handle.IsClosed ? null : CompletionQueueSafeHandle.CreateSync()) { bool callStartedOk = false; try diff --git a/src/csharp/tests.json b/src/csharp/tests.json index 4b0018e24893f..0ed3a2df61683 100644 --- a/src/csharp/tests.json +++ b/src/csharp/tests.json @@ -22,6 +22,7 @@ "Grpc.Core.Tests.AppDomainUnloadTest", "Grpc.Core.Tests.AuthContextTest", "Grpc.Core.Tests.AuthPropertyTest", + "Grpc.Core.Tests.CallAfterShutdownTest", "Grpc.Core.Tests.CallCancellationTest", "Grpc.Core.Tests.CallCredentialsTest", "Grpc.Core.Tests.CallOptionsTest",