diff --git a/src/Grpc.Net.Client/Internal/GrpcCall.cs b/src/Grpc.Net.Client/Internal/GrpcCall.cs index 162ceb5cc..69b15608f 100644 --- a/src/Grpc.Net.Client/Internal/GrpcCall.cs +++ b/src/Grpc.Net.Client/Internal/GrpcCall.cs @@ -23,6 +23,7 @@ using Grpc.Net.Client.Internal.Http; using Grpc.Shared; using Microsoft.Extensions.Logging; +using System.Threading.Tasks; #if SUPPORT_LOAD_BALANCING using Grpc.Net.Client.Balancer.Internal; #endif @@ -182,6 +183,10 @@ public void Dispose() Disposed = true; Cleanup(GrpcProtocolConstants.DisposeCanceledStatus); + + // If the call was disposed then observe any potential response exception. + // Observe the task's exception to prevent TaskScheduler.UnobservedTaskException from firing. + _responseTcs?.Task.ObserveException(); } } @@ -316,9 +321,21 @@ private async Task GetResponseHeadersCoreAsync() return metadata; } - catch (Exception ex) when (ResolveException(ErrorStartingCallMessage, ex, out _, out var resolvedException)) + catch (Exception ex) { - throw resolvedException; + // If there was an error fetching response headers then it's likely the same error is reported + // by response TCS. The user is unlikely to observe both errors. + // Observe the task's exception to prevent TaskScheduler.UnobservedTaskException from firing. + _responseTcs?.Task.ObserveException(); + + if (ResolveException(ErrorStartingCallMessage, ex, out _, out var resolvedException)) + { + throw resolvedException; + } + else + { + throw; + } } } @@ -584,13 +601,23 @@ private async Task RunCall(HttpRequestMessage request, TimeSpan? timeout) // Update HTTP response TCS before clean up. Needs to happen first because cleanup will // cancel the TCS for anyone still listening. _httpResponseTcs.TrySetException(resolvedException); + _httpResponseTcs.Task.ObserveException(); Cleanup(status.Value); // Update response TCS after overall call status is resolved. This is required so that // the call is completed before an error is thrown from ResponseAsync. If it happens // afterwards then there is a chance GetStatus() will error because the call isn't complete. - _responseTcs?.TrySetException(resolvedException); + if (_responseTcs != null) + { + _responseTcs.TrySetException(resolvedException); + + // Always observe cancellation-like exceptions. + if (IsCancellationOrDeadlineException(ex)) + { + _responseTcs.Task.ObserveException(); + } + } } // Verify that FinishCall is called in every code path of this method. diff --git a/src/Grpc.Net.Client/Internal/TaskExtensions.cs b/src/Grpc.Net.Client/Internal/TaskExtensions.cs new file mode 100644 index 000000000..ce800f03f --- /dev/null +++ b/src/Grpc.Net.Client/Internal/TaskExtensions.cs @@ -0,0 +1,51 @@ +#region Copyright notice and license + +// Copyright 2019 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 + +namespace Grpc.Net.Client.Internal +{ + internal static class TaskExtensions + { + private static readonly Action IgnoreTaskContinuation = t => { _ = t.Exception; }; + + /// + /// Observes and ignores a potential exception on a given Task. + /// If a Task fails and throws an exception which is never observed, it will be caught by the .NET finalizer thread. + /// This function awaits the given task and if the exception is thrown, it observes this exception and simply ignores it. + /// This will prevent the escalation of this exception to the .NET finalizer thread. + /// + /// The task to be ignored. + public static void ObserveException(this Task task) + { + if (task.IsCompleted) + { + if (task.IsFaulted) + { + _ = task.Exception; + } + } + else + { + task.ContinueWith( + IgnoreTaskContinuation, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } + } + } +} diff --git a/test/Grpc.Net.Client.Tests/AsyncUnaryCallTests.cs b/test/Grpc.Net.Client.Tests/AsyncUnaryCallTests.cs index 0e3457103..33eb88e11 100644 --- a/test/Grpc.Net.Client.Tests/AsyncUnaryCallTests.cs +++ b/test/Grpc.Net.Client.Tests/AsyncUnaryCallTests.cs @@ -24,6 +24,8 @@ using Grpc.Net.Client.Tests.Infrastructure; using Grpc.Shared; using Grpc.Tests.Shared; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using NUnit.Framework; namespace Grpc.Net.Client.Tests @@ -227,5 +229,89 @@ public async Task AsyncUnaryCall_SuccessTrailersOnly_ThrowNoMessageError() Assert.AreEqual(0, headers.Count); Assert.AreEqual(0, call.GetTrailers().Count); } + + public enum ResponseHandleAction + { + ResponseAsync, + ResponseHeadersAsync, + Dispose, + Nothing + } + + [Test] + [TestCase(0, ResponseHandleAction.ResponseAsync)] + [TestCase(0, ResponseHandleAction.ResponseHeadersAsync)] + [TestCase(0, ResponseHandleAction.Dispose)] + [TestCase(1, ResponseHandleAction.Nothing)] + public async Task AsyncUnaryCall_CallFailed_NoUnobservedExceptions(int expectedUnobservedExceptions, ResponseHandleAction action) + { + // Arrange + var services = new ServiceCollection(); + services.AddNUnitLogger(); + var loggerFactory = services.BuildServiceProvider().GetRequiredService(); + var logger = loggerFactory.CreateLogger(); + + var unobservedExceptions = new List(); + EventHandler onUnobservedTaskException = (sender, e) => + { + unobservedExceptions.Add(e.Exception!); + + logger.LogCritical(e.Exception!, "Unobserved task exception. Observed: " + e.Observed); + }; + + TaskScheduler.UnobservedTaskException += onUnobservedTaskException; + + try + { + var httpClient = ClientTestHelpers.CreateTestClient(request => + { + throw new Exception("Test error"); + }); + var invoker = HttpClientCallInvokerFactory.Create(httpClient, loggerFactory: loggerFactory); + + // Act + logger.LogDebug("Starting call"); + await MakeGrpcCallAsync(logger, invoker, action); + + logger.LogDebug("Waiting for finalizers"); + // Provoke the garbage collector to find the unobserved exception. + GC.Collect(); + // Wait for any failed tasks to be garbage collected + GC.WaitForPendingFinalizers(); + + // Assert + Assert.AreEqual(expectedUnobservedExceptions, unobservedExceptions.Count); + + static async Task MakeGrpcCallAsync(ILogger logger, HttpClientCallInvoker invoker, ResponseHandleAction action) + { + var runTask = Task.Run(async () => + { + var call = invoker.AsyncUnaryCall(ClientTestHelpers.ServiceMethod, string.Empty, new CallOptions(), new HelloRequest()); + + switch (action) + { + case ResponseHandleAction.ResponseAsync: + await ExceptionAssert.ThrowsAsync(() => call.ResponseAsync); + break; + case ResponseHandleAction.ResponseHeadersAsync: + await ExceptionAssert.ThrowsAsync(() => call.ResponseHeadersAsync); + break; + case ResponseHandleAction.Dispose: + call.Dispose(); + break; + default: + // Do nothing. + break; + } + }); + + await runTask; + } + } + finally + { + TaskScheduler.UnobservedTaskException -= onUnobservedTaskException; + } + } } } diff --git a/test/Grpc.Net.Client.Tests/Retry/HedgingCallTests.cs b/test/Grpc.Net.Client.Tests/Retry/HedgingCallTests.cs index 862b6f322..9c5855230 100644 --- a/test/Grpc.Net.Client.Tests/Retry/HedgingCallTests.cs +++ b/test/Grpc.Net.Client.Tests/Retry/HedgingCallTests.cs @@ -139,8 +139,6 @@ public async Task ActiveCalls_FatalStatusCode_CleansUpActiveCalls() // Fatal status code will cancel other calls Assert.AreEqual(0, hedgingCall._activeCalls.Count); await hedgingCall.CreateHedgingCallsTask!.DefaultTimeout(); - - waitUntilFinishedTcs.SetResult(null); } [Test]