From bc345bc8e7846195d24563b8bffbf311eac7fa31 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Mon, 25 Jun 2018 08:46:16 -0700 Subject: [PATCH] Reduce usage of static inprocess application (#962) --- .../managedexports.cxx | 47 +++++++++++----- .../Core/IISConfigurationData.cs | 4 +- .../Core/IISHttpServer.cs | 54 +++++++++++-------- .../Core/IISNativeApplication.cs | 48 +++++++++++++++++ .../NativeMethods.cs | 18 +++---- .../WebHostBuilderIISExtensions.cs | 1 + .../Inprocess/AppOfflineTests.cs | 2 +- .../Inprocess/ShutdownTests.cs | 31 +++++++++++ .../Utilities/Helpers.cs | 3 ++ test/WebSites/InProcessWebSite/Startup.cs | 11 ++++ 10 files changed, 172 insertions(+), 47 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Server.IIS/Core/IISNativeApplication.cs create mode 100644 test/IISIntegration.FunctionalTests/Inprocess/ShutdownTests.cs diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cxx b/src/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cxx index 3c82bd850a..9bc3c9de00 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cxx +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cxx @@ -9,8 +9,9 @@ // Initialization export // EXTERN_C __MIDL_DECLSPEC_DLLEXPORT -VOID +HRESULT register_callbacks( + _In_ IN_PROCESS_APPLICATION* pInProcessApplication, _In_ PFN_REQUEST_HANDLER request_handler, _In_ PFN_SHUTDOWN_HANDLER shutdown_handler, _In_ PFN_MANAGED_CONTEXT_HANDLER async_completion_handler, @@ -18,13 +19,20 @@ register_callbacks( _In_ VOID* pvShutdownHandlerContext ) { - IN_PROCESS_APPLICATION::GetInstance()->SetCallbackHandles( + if (pInProcessApplication == NULL) + { + return E_INVALIDARG; + } + + pInProcessApplication->SetCallbackHandles( request_handler, shutdown_handler, async_completion_handler, pvRequstHandlerContext, pvShutdownHandlerContext ); + + return S_OK; } EXTERN_C __MIDL_DECLSPEC_DLLEXPORT @@ -150,12 +158,12 @@ http_get_completion_info( } // -// todo: we should not rely on IN_PROCESS_APPLICATION::GetInstance() // the signature should be changed. application's based address should be passed in // struct IISConfigurationData { + IN_PROCESS_APPLICATION* pInProcessApplication; BSTR pwzFullApplicationPath; BSTR pwzVirtualApplicationPath; BOOL fWindowsAuthEnabled; @@ -169,16 +177,15 @@ http_get_application_properties( _In_ IISConfigurationData* pIISCofigurationData ) { - REQUESTHANDLER_CONFIG* pConfiguration = NULL; - IN_PROCESS_APPLICATION* pApplication = IN_PROCESS_APPLICATION::GetInstance(); - - if (pApplication == NULL) + auto pInProcessApplication = IN_PROCESS_APPLICATION::GetInstance(); + if (pInProcessApplication == NULL) { return E_FAIL; } - pConfiguration = pApplication->QueryConfig(); + auto pConfiguration = pInProcessApplication->QueryConfig(); + pIISCofigurationData->pInProcessApplication = pInProcessApplication; pIISCofigurationData->pwzFullApplicationPath = SysAllocString(pConfiguration->QueryApplicationPhysicalPath()->QueryStr()); pIISCofigurationData->pwzVirtualApplicationPath = SysAllocString(pConfiguration->QueryApplicationVirtualPath()->QueryStr()); pIISCofigurationData->fWindowsAuthEnabled = pConfiguration->QueryWindowsAuthEnabled(); @@ -433,17 +440,29 @@ http_get_authentication_information( } EXTERN_C __MIDL_DECLSPEC_DLLEXPORT -VOID -http_stop_calls_into_managed() +HRESULT +http_stop_calls_into_managed(_In_ IN_PROCESS_APPLICATION* pInProcessApplication) { - IN_PROCESS_APPLICATION::GetInstance()->StopCallsIntoManaged(); + if (pInProcessApplication == NULL) + { + return E_INVALIDARG; + } + + pInProcessApplication->StopCallsIntoManaged(); + return S_OK; } EXTERN_C __MIDL_DECLSPEC_DLLEXPORT -VOID -http_stop_incoming_requests() +HRESULT +http_stop_incoming_requests(_In_ IN_PROCESS_APPLICATION* pInProcessApplication) { - IN_PROCESS_APPLICATION::GetInstance()->StopIncomingRequests(); + if (pInProcessApplication == NULL) + { + return E_INVALIDARG; + } + + pInProcessApplication->StopIncomingRequests(); + return S_OK; } EXTERN_C __MIDL_DECLSPEC_DLLEXPORT diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISConfigurationData.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISConfigurationData.cs index 8f1f473766..5c55267756 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IISConfigurationData.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISConfigurationData.cs @@ -1,13 +1,15 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Runtime.InteropServices; namespace Microsoft.AspNetCore.Server.IIS.Core { [StructLayout(LayoutKind.Sequential)] - internal unsafe struct IISConfigurationData + internal struct IISConfigurationData { + public IntPtr pNativeApplication; [MarshalAs(UnmanagedType.BStr)] public string pwzFullApplicationPath; [MarshalAs(UnmanagedType.BStr)] diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs index 64c9eaf33a..1e6a7c86f4 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISHttpServer.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Http.Features; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Server.IIS.Core @@ -27,8 +28,9 @@ namespace Microsoft.AspNetCore.Server.IIS.Core private readonly MemoryPool _memoryPool = new SlabMemoryPool(); private GCHandle _httpServerHandle; private readonly IApplicationLifetime _applicationLifetime; - private readonly IAuthenticationSchemeProvider _authentication; + private readonly ILogger _logger; private readonly IISServerOptions _options; + private readonly IISNativeApplication _nativeApplication; private volatile int _stopping; private bool Stopping => _stopping == 1; @@ -56,10 +58,17 @@ namespace Microsoft.AspNetCore.Server.IIS.Core return _websocketAvailable.Value; } - public IISHttpServer(IApplicationLifetime applicationLifetime, IAuthenticationSchemeProvider authentication, IOptions options) + public IISHttpServer( + IISNativeApplication nativeApplication, + IApplicationLifetime applicationLifetime, + IAuthenticationSchemeProvider authentication, + IOptions options, + ILogger logger + ) { + _nativeApplication = nativeApplication; _applicationLifetime = applicationLifetime; - _authentication = authentication; + _logger = logger; _options = options.Value; if (_options.ForwardWindowsAuthentication) @@ -73,10 +82,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core _httpServerHandle = GCHandle.Alloc(this); _iisContextFactory = new IISContextFactory(_memoryPool, application, _options, this); - - // Start the server by registering the callback - NativeMethods.HttpRegisterCallbacks(_requestHandler, _shutdownHandler, _onAsyncCompletion, (IntPtr)_httpServerHandle, (IntPtr)_httpServerHandle); - + _nativeApplication.RegisterCallbacks(_requestHandler, _shutdownHandler, _onAsyncCompletion, (IntPtr)_httpServerHandle, (IntPtr)_httpServerHandle); return Task.CompletedTask; } @@ -86,7 +92,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core { cancellationToken.Register(() => { - NativeMethods.HttpStopCallsIntoManaged(); + _nativeApplication.StopCallsIntoManaged(); _shutdownSignal.TrySetResult(null); }); } @@ -98,7 +104,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core } // First call back into native saying "DON'T SEND ME ANY MORE REQUESTS" - NativeMethods.HttpStopIncomingRequests(); + _nativeApplication.StopIncomingRequests(); try { @@ -110,7 +116,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core else { // We have drained all requests. Block any callbacks into managed at this point. - NativeMethods.HttpStopCallsIntoManaged(); + _nativeApplication.StopCallsIntoManaged(); _shutdownSignal.TrySetResult(null); } } @@ -127,7 +133,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core _stopping = 1; // Block any more calls into managed from native as we are unloading. - NativeMethods.HttpStopCallsIntoManaged(); + _nativeApplication.StopCallsIntoManaged(); _shutdownSignal.TrySetResult(null); if (_httpServerHandle.IsAllocated) @@ -136,8 +142,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core } _memoryPool.Dispose(); - - GC.SuppressFinalize(this); + _nativeApplication.Dispose(); } private static NativeMethods.REQUEST_NOTIFICATION_STATUS HandleRequest(IntPtr pInProcessHandler, IntPtr pvRequestContext) @@ -155,8 +160,19 @@ namespace Microsoft.AspNetCore.Server.IIS.Core private static async Task HandleRequest(IISHttpContext context) { - var result = await context.ProcessRequestAsync(); - CompleteRequest(context, result); + bool successfulRequest = false; + try + { + successfulRequest = await context.ProcessRequestAsync(); + } + catch (Exception ex) + { + context.Server._logger.LogError("Exception in ProcessRequestAsync", ex); + } + finally + { + CompleteRequest(context, successfulRequest); + } } private static bool HandleShutdown(IntPtr pvRequestContext) @@ -181,7 +197,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core if (Interlocked.Decrement(ref context.Server._outstandingRequests) == 0 && context.Server.Stopping) { // All requests have been drained. - NativeMethods.HttpStopCallsIntoManaged(); + context.Server._nativeApplication.StopCallsIntoManaged(); context.Server._shutdownSignal.TrySetResult(null); } @@ -215,12 +231,6 @@ namespace Microsoft.AspNetCore.Server.IIS.Core return new IISHttpContextOfT(_memoryPool, _application, pInProcessHandler, _options, _server); } } - - ~IISHttpServer() - { - // If this finalize is invoked, try our best to block all calls into managed. - NativeMethods.HttpStopCallsIntoManaged(); - } } // Over engineering to avoid allocations... diff --git a/src/Microsoft.AspNetCore.Server.IIS/Core/IISNativeApplication.cs b/src/Microsoft.AspNetCore.Server.IIS/Core/IISNativeApplication.cs new file mode 100644 index 0000000000..6d574867b4 --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.IIS/Core/IISNativeApplication.cs @@ -0,0 +1,48 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; + +namespace Microsoft.AspNetCore.Server.IIS.Core +{ + internal class IISNativeApplication + { + private readonly IntPtr _nativeApplication; + + public IISNativeApplication(IntPtr nativeApplication) + { + _nativeApplication = nativeApplication; + } + + public void StopIncomingRequests() + { + NativeMethods.HttpStopIncomingRequests(_nativeApplication); + } + + public void StopCallsIntoManaged() + { + NativeMethods.HttpStopCallsIntoManaged(_nativeApplication); + } + + public void RegisterCallbacks( + NativeMethods.PFN_REQUEST_HANDLER requestHandler, + NativeMethods.PFN_SHUTDOWN_HANDLER shutdownHandler, + NativeMethods.PFN_ASYNC_COMPLETION onAsyncCompletion, + IntPtr requestContext, + IntPtr shutdownContext) + { + NativeMethods.HttpRegisterCallbacks(_nativeApplication, requestHandler, shutdownHandler, onAsyncCompletion, requestContext, shutdownContext); + } + + public void Dispose() + { + GC.SuppressFinalize(this); + } + + ~IISNativeApplication() + { + // If this finalize is invoked, try our best to block all calls into managed. + StopCallsIntoManaged(); + } + } +} diff --git a/src/Microsoft.AspNetCore.Server.IIS/NativeMethods.cs b/src/Microsoft.AspNetCore.Server.IIS/NativeMethods.cs index bd4a31db78..55b8084e59 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/NativeMethods.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/NativeMethods.cs @@ -53,7 +53,7 @@ namespace Microsoft.AspNetCore.Server.IIS private static extern void http_indicate_completion(IntPtr pInProcessHandler, REQUEST_NOTIFICATION_STATUS notificationStatus); [DllImport(AspNetCoreModuleDll)] - private static extern void register_callbacks(PFN_REQUEST_HANDLER request_callback, PFN_SHUTDOWN_HANDLER shutdown_callback, PFN_ASYNC_COMPLETION managed_context_handler, IntPtr pvRequestContext, IntPtr pvShutdownContext); + private static extern int register_callbacks(IntPtr pInProcessApplication, PFN_REQUEST_HANDLER requestCallback, PFN_SHUTDOWN_HANDLER shutdownCallback, PFN_ASYNC_COMPLETION asyncCallback, IntPtr pvRequestContext, IntPtr pvShutdownContext); [DllImport(AspNetCoreModuleDll)] private static extern unsafe int http_write_response_bytes(IntPtr pInProcessHandler, HttpApiTypes.HTTP_DATA_CHUNK* pDataChunks, int nChunks, out bool fCompletionExpected); @@ -65,10 +65,10 @@ namespace Microsoft.AspNetCore.Server.IIS private static extern unsafe HttpApiTypes.HTTP_REQUEST_V2* http_get_raw_request(IntPtr pInProcessHandler); [DllImport(AspNetCoreModuleDll)] - private static extern void http_stop_calls_into_managed(); + private static extern int http_stop_calls_into_managed(IntPtr pInProcessApplication); [DllImport(AspNetCoreModuleDll)] - private static extern void http_stop_incoming_requests(); + private static extern int http_stop_incoming_requests(IntPtr pInProcessApplication); [DllImport(AspNetCoreModuleDll)] private static extern unsafe HttpApiTypes.HTTP_RESPONSE_V2* http_get_raw_response(IntPtr pInProcessHandler); @@ -142,9 +142,9 @@ namespace Microsoft.AspNetCore.Server.IIS { http_indicate_completion(pInProcessHandler, notificationStatus); } - public static void HttpRegisterCallbacks(PFN_REQUEST_HANDLER request_callback, PFN_SHUTDOWN_HANDLER shutdown_callback, PFN_ASYNC_COMPLETION managed_context_handler, IntPtr pvRequestContext, IntPtr pvShutdownContext) + public static void HttpRegisterCallbacks(IntPtr pInProcessApplication, PFN_REQUEST_HANDLER requestCallback, PFN_SHUTDOWN_HANDLER shutdownCallback, PFN_ASYNC_COMPLETION asyncCallback, IntPtr pvRequestContext, IntPtr pvShutdownContext) { - register_callbacks(request_callback, shutdown_callback, managed_context_handler, pvRequestContext, pvShutdownContext); + Validate(register_callbacks(pInProcessApplication, requestCallback, shutdownCallback, asyncCallback, pvRequestContext, pvShutdownContext)); } public static unsafe int HttpWriteResponseBytes(IntPtr pInProcessHandler, HttpApiTypes.HTTP_DATA_CHUNK* pDataChunks, int nChunks, out bool fCompletionExpected) @@ -161,14 +161,14 @@ namespace Microsoft.AspNetCore.Server.IIS return http_get_raw_request(pInProcessHandler); } - public static void HttpStopCallsIntoManaged() + public static void HttpStopCallsIntoManaged(IntPtr pInProcessApplication) { - http_stop_calls_into_managed(); + Validate(http_stop_calls_into_managed(pInProcessApplication)); } - public static void HttpStopIncomingRequests() + public static void HttpStopIncomingRequests(IntPtr pInProcessApplication) { - http_stop_incoming_requests(); + Validate(http_stop_incoming_requests(pInProcessApplication)); } public static unsafe HttpApiTypes.HTTP_RESPONSE_V2* HttpGetRawResponse(IntPtr pInProcessHandler) diff --git a/src/Microsoft.AspNetCore.Server.IIS/WebHostBuilderIISExtensions.cs b/src/Microsoft.AspNetCore.Server.IIS/WebHostBuilderIISExtensions.cs index e5ca048afc..6ce99f7832 100644 --- a/src/Microsoft.AspNetCore.Server.IIS/WebHostBuilderIISExtensions.cs +++ b/src/Microsoft.AspNetCore.Server.IIS/WebHostBuilderIISExtensions.cs @@ -35,6 +35,7 @@ namespace Microsoft.AspNetCore.Hosting hostBuilder.UseContentRoot(iisConfigData.pwzFullApplicationPath); return hostBuilder.ConfigureServices( services => { + services.AddSingleton(new IISNativeApplication(iisConfigData.pNativeApplication)); services.AddSingleton(); services.AddSingleton(new IISServerSetupFilter(iisConfigData.pwzVirtualApplicationPath)); services.AddAuthenticationCore(); diff --git a/test/IISIntegration.FunctionalTests/Inprocess/AppOfflineTests.cs b/test/IISIntegration.FunctionalTests/Inprocess/AppOfflineTests.cs index b5ac2758e0..d88fad5d21 100644 --- a/test/IISIntegration.FunctionalTests/Inprocess/AppOfflineTests.cs +++ b/test/IISIntegration.FunctionalTests/Inprocess/AppOfflineTests.cs @@ -114,7 +114,7 @@ namespace IISIntegration.FunctionalTests.Inprocess var hostShutdownToken = deploymentResult.DeploymentResult.HostShutdownToken; - Assert.True(hostShutdownToken.WaitHandle.WaitOne(millisecondsTimeout: 1000)); + Assert.True(hostShutdownToken.WaitHandle.WaitOne(Helpers.DefaultTimeout)); Assert.True(hostShutdownToken.IsCancellationRequested); } diff --git a/test/IISIntegration.FunctionalTests/Inprocess/ShutdownTests.cs b/test/IISIntegration.FunctionalTests/Inprocess/ShutdownTests.cs new file mode 100644 index 0000000000..9b1eb28eab --- /dev/null +++ b/test/IISIntegration.FunctionalTests/Inprocess/ShutdownTests.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using IISIntegration.FunctionalTests.Utilities; +using Microsoft.AspNetCore.Server.IntegrationTesting; +using Microsoft.AspNetCore.Testing.xunit; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests +{ + [SkipIfIISExpressSchemaMissingInProcess] + public class ShutdownTests : IISFunctionalTestBase + { + + public ShutdownTests(ITestOutputHelper output) : base(output) + { + } + + [ConditionalFact] + public async Task ServerShutsDownWhenMainExits() + { + var parameters = Helpers.GetBaseDeploymentParameters(); + var result = await DeployAsync(parameters); + + var response = await result.RetryingHttpClient.GetAsync("/Shutdown"); + Assert.True(result.DeploymentResult.HostShutdownToken.WaitHandle.WaitOne(Helpers.DefaultTimeout)); + } + } +} diff --git a/test/IISIntegration.FunctionalTests/Utilities/Helpers.cs b/test/IISIntegration.FunctionalTests/Utilities/Helpers.cs index d66fecf3cb..cfee3377f2 100644 --- a/test/IISIntegration.FunctionalTests/Utilities/Helpers.cs +++ b/test/IISIntegration.FunctionalTests/Utilities/Helpers.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.IO; using System.Linq; using System.Xml.Linq; @@ -11,6 +12,8 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { public class Helpers { + public static TimeSpan DefaultTimeout = TimeSpan.FromSeconds(3); + public static string GetTestWebSitePath(string name) { return Path.Combine(TestPathUtilities.GetSolutionRootDirectory("IISIntegration"),"test", "WebSites", name); diff --git a/test/WebSites/InProcessWebSite/Startup.cs b/test/WebSites/InProcessWebSite/Startup.cs index 8a190d47f0..8d0e49d946 100644 --- a/test/WebSites/InProcessWebSite/Startup.cs +++ b/test/WebSites/InProcessWebSite/Startup.cs @@ -8,10 +8,12 @@ using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.IISIntegration.FunctionalTests; using Microsoft.AspNetCore.Server.IIS; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Primitives; using Xunit; @@ -725,5 +727,14 @@ namespace IISTestSite }); } + private void Shutdown(IApplicationBuilder app) + { + app.Run(async ctx => + { + await ctx.Response.WriteAsync("Shutting down"); + ctx.RequestServices.GetService().StopApplication(); + }); + } + } }