Conversation
github-actions
bot
added
the
area-networking
There was a problem hiding this comment.
Pull request overview
Fixes a shutdown hang when IIS has Preload Enabled = true by introducing a custom IISHostLifetime that avoids the 30-second blocking sleep added in .NET 10.0's ConsoleLifetime. Also fixes a null reference bug in IISDeployer.
Changes:
- Adds
IISHostLifetimeas a replacement forConsoleLifetimein the IIS in-process hosting path - Fixes null dereference in
IISDeployer.WaitUntilSiteStartedby movingsite.Nameaccess after the null check - Adds tests verifying the correct
IHostLifetimeis used for in-process and out-of-process hosting
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| WebHostBuilderIISExtensions.cs | Registers IISHostLifetime and defines the class |
| IISDeployer.cs | Moves site.Name access after null check |
| StartupTests.cs | Adds tests for host lifetime verification |
| InProcessWebSite/Program.cs | Adds HostBuilder test case |
| InProcessWebSite/Startup.cs | Adds GetHostLifetime endpoint |
| [ConditionalFact] | ||
| public async Task OutOfProcessHostlifetime() | ||
| { | ||
| if (DeployerSelector.IsNewShimTest) | ||
| { | ||
| // NewShim tests use 2.2 IIS packages which don't have the host lifetime | ||
| return; | ||
| } | ||
|
|
||
| var deploymentParameters = Fixture.GetBaseDeploymentParameters(HostingModel.OutOfProcess); | ||
| deploymentParameters.TransformArguments((a, _) => $"{a} HostBuilder"); | ||
|
|
||
| if (deploymentParameters.ServerType == ServerType.IISExpress) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| Assert.Equal("ConsoleLifetime", await GetStringAsync(deploymentParameters, "GetHostLifetime")); |
There was a problem hiding this comment.
The early return for IISExpress silently skips the test without any indication. Consider using Skip or a ConditionalFact attribute so test results clearly show the test was skipped rather than appearing to pass.
| } | ||
|
|
||
| [ConditionalFact] | ||
| public async Task InProcessHostlifetime() |
There was a problem hiding this comment.
Minor inconsistency: Hostlifetime should be HostLifetime (capital 'L') to match .NET naming conventions.
| // for SIGTERM on Windows we must block this thread until the application is finished | ||
| // otherwise the process will be killed immediately on return from this handler | ||
|
|
||
| // don't allow Dispose to unregister handlers, since Windows has a lock that prevents the unregistration while this handler is running |
There was a problem hiding this comment.
Note that this code is changing in dotnet/runtime#124893
|
|
||
| public Task WaitForStartAsync(CancellationToken cancellationToken) | ||
| { | ||
| _sigTermRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, |
There was a problem hiding this comment.
I assume that it is SIGINT and SIGQUIT handlers are intentionally missing here since they are not needed by IIS.
There was a problem hiding this comment.
Right, ctrl-c or ctrl-break doesn't make sense for IIS apps.
| // Does not copy Thread.Sleep(HostOptions.ShutdownTimeout) | ||
| // from ConsoleLifetime because that causes machine shutdown to hang | ||
| // if Preload Enabled = true | ||
| // Likely an IIS/WAS issue. |
There was a problem hiding this comment.
This is fine as a minimal low-risk fix that can be backported.
For the right fix, I would expect that there is a IIS-native way to subscribe to shutdown and we should not need PosixSignal.SIGTERM handler at all.
There was a problem hiding this comment.
Yeah, I hope we can find a better solution in the future and this is just a temporary solution for customers that are broken by the current behavior.
|
/backport to release/10.0 |
|
Started backporting to |
Fixes #65436
dotnet/runtime#116652 revealed what is probably a latent bug in IIS/WAS/Windows where if
Preload Enabled = truefor your IIS application then machine shutdown can hang forever due to the default 30 second delay blocking the w3wp process from being closed which allows a new w3wp process time to start up, rinse and repeat.This PR is using a custom
IHostLifetimethat effectively revertsConsoleLifetimeto the pre-10.0 behavior of not blocking for 30 seconds to workaround whatever bug exists in the lower layers. I have pinged an IIS dev about this, and will push on a solution from that end outside of this change.