feedback and add another shutdown test file

Author: jkotalik

src/Servers/IIS/IIS/src/Core/IISHttpServer.cs

@@ -25,7 +25,7 @@ internal class IISHttpServer : IServer
         private static readonly NativeMethods.PFN_SHUTDOWN_HANDLER _shutdownHandler = HandleShutdown;
         private static readonly NativeMethods.PFN_DISCONNECT_HANDLER _onDisconnect = OnDisconnect;
         private static readonly NativeMethods.PFN_ASYNC_COMPLETION _onAsyncCompletion = OnAsyncCompletion;
-        private static readonly NativeMethods.PFN_REQUESTS_DRAINED_HANDLER _drainHandler = OnRequestsDrained;
+        private static readonly NativeMethods.PFN_REQUESTS_DRAINED_HANDLER _requestsDrainedHandler = OnRequestsDrained;
 
         private IISContextFactory _iisContextFactory;
         private readonly MemoryPool<byte> _memoryPool = new SlabMemoryPool();
@@ -36,8 +36,6 @@ internal class IISHttpServer : IServer
         private readonly IISNativeApplication _nativeApplication;
         private readonly ServerAddressesFeature _serverAddressesFeature;
 
-        private CancellationTokenRegistration _stopRegistration;
-
         private readonly TaskCompletionSource<object> _shutdownSignal = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
         private bool? _websocketAvailable;
 
@@ -88,7 +86,7 @@ public Task StartAsync<TContext>(IHttpApplication<TContext> application, Cancell
             _httpServerHandle = GCHandle.Alloc(this);
 
             _iisContextFactory = new IISContextFactory<TContext>(_memoryPool, application, _options, this, _logger);
-            _nativeApplication.RegisterCallbacks(_requestHandler, _shutdownHandler, _onDisconnect, _onAsyncCompletion, _drainHandler, (IntPtr)_httpServerHandle, (IntPtr)_httpServerHandle);
+            _nativeApplication.RegisterCallbacks(_requestHandler, _shutdownHandler, _onDisconnect, _onAsyncCompletion, _requestsDrainedHandler, (IntPtr)_httpServerHandle, (IntPtr)_httpServerHandle);
 
             _serverAddressesFeature.Addresses = _options.ServerAddresses;
 
@@ -99,18 +97,6 @@ public Task StopAsync(CancellationToken cancellationToken)
         {
             _nativeApplication.StopIncomingRequests();
 
-            try
-            {
-                _stopRegistration = cancellationToken.Register(() =>
-                {
-                    _shutdownSignal.TrySetResult(null);
-                });
-            }
-            catch (Exception ex)
-            {
-                _shutdownSignal.TrySetException(ex);
-            }
-
             return _shutdownSignal.Task;
         }
 
@@ -206,13 +192,13 @@ private static void OnRequestsDrained(IntPtr serverContext)
 
                 server._nativeApplication.StopCallsIntoManaged();
                 server._shutdownSignal.TrySetResult(null);
-                server._stopRegistration.Dispose();
             }
             catch (Exception ex)
             {
                 server?._logger.LogError(0, ex, $"Unexpected exception in {nameof(IISHttpServer)}.{nameof(OnRequestsDrained)}.");
             }
         }
+
         private class IISContextFactory<T> : IISContextFactory
         {
             private readonly IHttpApplication<T> _application;

src/Servers/IIS/IIS/test/Common.FunctionalTests/Inprocess/ShutdownTests.cs

@@ -0,0 +1,65 @@
+
+// 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.IO;
+using System.Net.Http;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities;
+using Microsoft.AspNetCore.Server.IntegrationTesting.IIS;
+using Microsoft.AspNetCore.Testing.xunit;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests
+{
+    [Collection(PublishedSitesCollection.Name)]
+    public class IISExpressShutdownTests : IISFunctionalTestBase
+    {
+        private readonly PublishedSitesFixture _fixture;
+
+        public IISExpressShutdownTests(PublishedSitesFixture fixture)
+        {
+            _fixture = fixture;
+        }
+
+        [ConditionalFact]
+        public async Task ShutdownTimeoutIsApplied()
+        {
+            var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.InProcessTestSite, publish: true);
+            deploymentParameters.TransformArguments((a, _) => $"{a} HangOnStop");
+            deploymentParameters.WebConfigActionList.Add(
+                WebConfigHelpers.AddOrModifyAspNetCoreSection("shutdownTimeLimit", "1"));
+
+            var deploymentResult = await DeployAsync(deploymentParameters);
+
+            Assert.Equal("Hello World", await deploymentResult.HttpClient.GetStringAsync("/HelloWorld"));
+
+            StopServer();
+
+            EventLogHelpers.VerifyEventLogEvents(deploymentResult,
+                EventLogHelpers.InProcessStarted(deploymentResult),
+                EventLogHelpers.InProcessFailedToStop(deploymentResult, ""));
+        }
+
+        [ConditionalTheory]
+        [InlineData("/ShutdownStopAsync")]
+        [InlineData("/ShutdownStopAsyncWithCancelledToken")]
+        public async Task CallStopAsyncOnRequestThread_DoesNotHangIndefinitely(string path)
+        {
+            // Canceled token doesn't affect shutdown, in-proc doesn't handle ungraceful shutdown
+            // IIS's ShutdownTimeLimit will handle that.
+            var parameters = _fixture.GetBaseDeploymentParameters(publish: true);
+            var deploymentResult = await DeployAsync(parameters);
+            try
+            {
+                await deploymentResult.HttpClient.GetAsync(path);
+            }
+            catch (HttpRequestException ex) when (ex.InnerException is IOException)
+            {
+                // Server might close a connection before request completes
+            }
+
+            deploymentResult.AssertWorkerProcessStop();
+        }
+    }
+}

src/Servers/IIS/IIS/test/Common.FunctionalTests/Inprocess/StartupTests.cs

@@ -6,6 +6,7 @@
 using System.IO;
 using System.Linq;
 using System.Net;
+using System.Net.Http;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using System.Xml.Linq;
@@ -340,25 +341,6 @@ public async Task StartupTimeoutIsApplied()
                 );
         }
 
-        [ConditionalFact]
-        public async Task ShutdownTimeoutIsApplied()
-        {
-            var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.InProcessTestSite, publish: true);
-            deploymentParameters.TransformArguments((a, _) => $"{a} HangOnStop");
-            deploymentParameters.WebConfigActionList.Add(
-                WebConfigHelpers.AddOrModifyAspNetCoreSection("shutdownTimeLimit", "1"));
-
-            var deploymentResult = await DeployAsync(deploymentParameters);
-
-            Assert.Equal("Hello World", await deploymentResult.HttpClient.GetStringAsync("/HelloWorld"));
-
-            StopServer();
-
-            EventLogHelpers.VerifyEventLogEvents(deploymentResult,
-                EventLogHelpers.InProcessStarted(deploymentResult),
-                EventLogHelpers.InProcessFailedToStop(deploymentResult, ""));
-        }
-
         [ConditionalFact]
         public async Task CheckInvalidHostingModelParameter()
         {

src/Servers/IIS/IIS/test/IISExpress.FunctionalTests/InProcess/ShutdownTests.cs renamed to src/Servers/IIS/IIS/test/IISExpress.FunctionalTests/InProcess/IISExpressShutdownTests.cs

@@ -15,11 +15,11 @@
 namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests
 {
     [Collection(PublishedSitesCollection.Name)]
-    public class ShutdownTests : IISFunctionalTestBase
+    public class IISExpressShutdownTests : IISFunctionalTestBase
     {
         private readonly PublishedSitesFixture _fixture;
 
-        public ShutdownTests(PublishedSitesFixture fixture)
+        public IISExpressShutdownTests(PublishedSitesFixture fixture)
         {
             _fixture = fixture;
         }
@@ -87,24 +87,5 @@ public async Task ForcefulShutdown_DoesCrashProcess()
             StopServer(gracefulShutdown: false);
             Assert.True(result.HostProcess.ExitCode == 1);
         }
-
-        [ConditionalTheory]
-        [InlineData("/ShutdownStopAsync")]
-        [InlineData("/ShutdownStopAsyncWithCancelledToken")]
-        public async Task CallStopAsyncOnRequestThread_DoesNotHangIndefinitely(string path)
-        {
-            var parameters = _fixture.GetBaseDeploymentParameters(publish: true);
-            var deploymentResult = await DeployAsync(parameters);
-            try
-            {
-                await deploymentResult.HttpClient.GetAsync(path);
-            }
-            catch (HttpRequestException ex) when (ex.InnerException is IOException)
-            {
-                // Server might close a connection before request completes
-            }
-
-            deploymentResult.AssertWorkerProcessStop();
-        }
     }
 }