diff --git a/perf/Grpc.AspNetCore.Microbenchmarks/DefaultCoreConfig.cs b/perf/Grpc.AspNetCore.Microbenchmarks/DefaultCoreConfig.cs index fccd0912d..4646855ca 100644 --- a/perf/Grpc.AspNetCore.Microbenchmarks/DefaultCoreConfig.cs +++ b/perf/Grpc.AspNetCore.Microbenchmarks/DefaultCoreConfig.cs @@ -44,7 +44,7 @@ public DefaultCoreConfig() Add(JitOptimizationsValidator.FailOnError); Add(Job.Core - .With(CsProjCoreToolchain.From(new NetCoreAppSettings("netcoreapp3.0", null, ".NET Core 3.0"))) + .With(CsProjCoreToolchain.From(new NetCoreAppSettings("netcoreapp3.1", null, ".NET Core 3.1"))) .With(new GcMode { Server = true }) .With(RunStrategy.Throughput)); } diff --git a/src/Shared/Server/UnaryServerMethodInvoker.cs b/src/Shared/Server/UnaryServerMethodInvoker.cs index 5b817b1ed..f39009f90 100644 --- a/src/Shared/Server/UnaryServerMethodInvoker.cs +++ b/src/Shared/Server/UnaryServerMethodInvoker.cs @@ -16,6 +16,8 @@ #endregion +using System; +using System.Runtime.ExceptionServices; using System.Threading.Tasks; using Grpc.AspNetCore.Server; using Grpc.AspNetCore.Server.Model; @@ -86,33 +88,89 @@ private async Task ResolvedInterceptorInvoker(TRequest resolvedReques /// The message. /// A that represents the asynchronous method. The /// property returns the message. - public async Task Invoke(HttpContext httpContext, ServerCallContext serverCallContext, TRequest request) + public Task Invoke(HttpContext httpContext, ServerCallContext serverCallContext, TRequest request) { if (_pipelineInvoker == null) { GrpcActivatorHandle serviceHandle = default; + Task? invokerTask = null; try { serviceHandle = ServiceActivator.Create(httpContext.RequestServices); - return await _invoker( + invokerTask = _invoker( serviceHandle.Instance, request, serverCallContext); } - finally + catch (Exception ex) { + // Invoker calls user code. User code may throw an exception instead + // of a faulted task. We need to catch the exception, ensure cleanup + // runs and convert exception into a faulted task. if (serviceHandle.Instance != null) { - await ServiceActivator.ReleaseAsync(serviceHandle); + var releaseTask = ServiceActivator.ReleaseAsync(serviceHandle); + if (!releaseTask.IsCompletedSuccessfully) + { + // Capture the current exception state so we can rethrow it after awaiting + // with the same stack trace. + var exceptionDispatchInfo = ExceptionDispatchInfo.Capture(ex); + return AwaitServiceReleaseAndThrow(releaseTask, exceptionDispatchInfo); + } } + + return Task.FromException(ex); + } + + if (invokerTask.IsCompletedSuccessfully && serviceHandle.Instance != null) + { + var releaseTask = ServiceActivator.ReleaseAsync(serviceHandle); + if (!releaseTask.IsCompletedSuccessfully) + { + return AwaitServiceReleaseAndReturn(invokerTask.Result, serviceHandle); + } + + return invokerTask; } + + return AwaitInvoker(invokerTask, serviceHandle); } else { - return await _pipelineInvoker( + return _pipelineInvoker( request, serverCallContext); } } + + private async Task AwaitInvoker(Task invokerTask, GrpcActivatorHandle serviceHandle) + { + try + { + return await invokerTask; + } + finally + { + if (serviceHandle.Instance != null) + { + await ServiceActivator.ReleaseAsync(serviceHandle); + } + } + } + + private async Task AwaitServiceReleaseAndThrow(ValueTask releaseTask, ExceptionDispatchInfo ex) + { + await releaseTask; + ex.Throw(); + + // Should never reach here + return null; + } + + private async Task AwaitServiceReleaseAndReturn(TResponse invokerResult, GrpcActivatorHandle serviceHandle) + { + await ServiceActivator.ReleaseAsync(serviceHandle); + return invokerResult; + } } } diff --git a/test/Grpc.AspNetCore.Server.Tests/TestObjects/TestGrpcServiceActivator.cs b/test/Grpc.AspNetCore.Server.Tests/TestObjects/TestGrpcServiceActivator.cs index 9a7866ab0..3559317b3 100644 --- a/test/Grpc.AspNetCore.Server.Tests/TestObjects/TestGrpcServiceActivator.cs +++ b/test/Grpc.AspNetCore.Server.Tests/TestObjects/TestGrpcServiceActivator.cs @@ -23,6 +23,8 @@ namespace Grpc.AspNetCore.Server.Tests.TestObjects { internal class TestGrpcServiceActivator : IGrpcServiceActivator where TGrpcService : class, new() { + public bool Released { get; private set; } + public GrpcActivatorHandle Create(IServiceProvider serviceProvider) { return new GrpcActivatorHandle(new TGrpcService(), false, null); @@ -30,6 +32,7 @@ public GrpcActivatorHandle Create(IServiceProvider serviceProvider public ValueTask ReleaseAsync(GrpcActivatorHandle service) { + Released = true; return default; } } diff --git a/test/Grpc.AspNetCore.Server.Tests/UnaryServerCallHandlerTests.cs b/test/Grpc.AspNetCore.Server.Tests/UnaryServerCallHandlerTests.cs new file mode 100644 index 000000000..16137e2c1 --- /dev/null +++ b/test/Grpc.AspNetCore.Server.Tests/UnaryServerCallHandlerTests.cs @@ -0,0 +1,162 @@ +#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 + +using System; +using System.Threading.Tasks; +using Grpc.AspNetCore.Server.Tests.TestObjects; +using Grpc.Core; +using Grpc.Shared.Server; +using Grpc.Tests.Shared; +using NUnit.Framework; + +namespace Grpc.AspNetCore.Server.Tests +{ + [TestFixture] + public class UnaryServerCallHandlerTests + { + private static readonly Marshaller _marshaller = new Marshaller((message, context) => { context.Complete(Array.Empty()); }, context => new TestMessage()); + + [Test] + public void Invoke_ThrowException_ReleaseCalledAndErrorThrown() + { + // Arrange + var serviceActivator = new TestGrpcServiceActivator(); + var ex = new Exception("Exception!"); + var invoker = new UnaryServerMethodInvoker( + (service, reader, context) => throw ex, + new Method(MethodType.Unary, "test", "test", _marshaller, _marshaller), + HttpContextServerCallContextHelper.CreateMethodOptions(), + serviceActivator); + var httpContext = HttpContextHelpers.CreateContext(); + + // Act + var task = invoker.Invoke(httpContext, HttpContextServerCallContextHelper.CreateServerCallContext(), new TestMessage()); + + // Assert + Assert.True(serviceActivator.Released); + Assert.True(task.IsFaulted); + Assert.AreEqual(ex, task.Exception!.InnerException); + } + + [Test] + public async Task Invoke_ThrowExceptionAwaitedRelease_ReleaseCalledAndErrorThrown() + { + // Arrange + var releaseTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var serviceActivator = new TcsGrpcServiceActivator(releaseTcs); + var thrownException = new Exception("Exception!"); + var invoker = new UnaryServerMethodInvoker( + (service, reader, context) => throw thrownException, + new Method(MethodType.Unary, "test", "test", _marshaller, _marshaller), + HttpContextServerCallContextHelper.CreateMethodOptions(), + serviceActivator); + var httpContext = HttpContextHelpers.CreateContext(); + + // Act + var task = invoker.Invoke(httpContext, HttpContextServerCallContextHelper.CreateServerCallContext(), new TestMessage()); + Assert.False(task.IsCompleted); + + releaseTcs.SetResult(null); + + try + { + await task; + Assert.Fail(); + } + catch (Exception ex) + { + // Assert + Assert.True(serviceActivator.Released); + Assert.AreEqual(thrownException, ex); + } + } + + [Test] + public async Task Invoke_SuccessAwaitedRelease_ReleaseCalled() + { + // Arrange + var releaseTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var serviceActivator = new TcsGrpcServiceActivator(releaseTcs); + var invoker = new UnaryServerMethodInvoker( + (service, reader, context) => Task.FromResult(new TestMessage()), + new Method(MethodType.Unary, "test", "test", _marshaller, _marshaller), + HttpContextServerCallContextHelper.CreateMethodOptions(), + serviceActivator); + var httpContext = HttpContextHelpers.CreateContext(); + + // Act + var task = invoker.Invoke(httpContext, HttpContextServerCallContextHelper.CreateServerCallContext(), new TestMessage()); + Assert.False(task.IsCompleted); + + releaseTcs.SetResult(null); + await task; + + // Assert + Assert.True(serviceActivator.Released); + } + + [Test] + public async Task Invoke_AwaitedSuccess_ReleaseCalled() + { + // Arrange + var methodTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var methodResult = new TestMessage(); + var serviceActivator = new TestGrpcServiceActivator(); + var invoker = new UnaryServerMethodInvoker( + (service, reader, context) => methodTcs.Task, + new Method(MethodType.Unary, "test", "test", _marshaller, _marshaller), + HttpContextServerCallContextHelper.CreateMethodOptions(), + serviceActivator); + var httpContext = HttpContextHelpers.CreateContext(); + + // Act + var task = invoker.Invoke(httpContext, HttpContextServerCallContextHelper.CreateServerCallContext(), new TestMessage()); + Assert.False(task.IsCompleted); + + methodTcs.SetResult(methodResult); + var awaitedResult = await task; + + // Assert + Assert.AreEqual(methodResult, awaitedResult); + Assert.True(serviceActivator.Released); + } + + private class TcsGrpcServiceActivator : IGrpcServiceActivator where TGrpcService : class, new() + { + private readonly TaskCompletionSource _tcs; + + public bool Released { get; private set; } + + public TcsGrpcServiceActivator(TaskCompletionSource tcs) + { + _tcs = tcs; + } + + public GrpcActivatorHandle Create(IServiceProvider serviceProvider) + { + return new GrpcActivatorHandle(new TGrpcService(), false, null); + } + + public ValueTask ReleaseAsync(GrpcActivatorHandle service) + { + Released = true; + return new ValueTask(_tcs.Task); + } + } + } +}