Skip to content

Defensive cleanup in IIS request processing logic#66622

Merged
BrennanConroy merged 1 commit into
mainfrom
brecon/iisrequestcleanup
May 8, 2026
Merged

Defensive cleanup in IIS request processing logic#66622
BrennanConroy merged 1 commit into
mainfrom
brecon/iisrequestcleanup

Conversation

@BrennanConroy
Copy link
Copy Markdown
Member

While doing some random investigations and playing around with throwing from random places, I noticed that if

_bodyOutput.Complete();
_bodyInputPipe?.Reader.Complete();

// Allow writes to drain
if (_writeBodyTask != null)
{
    await _writeBodyTask;
}

// Cancel all remaining IO, there might be reads pending if not entire request body was sent by client
AsyncIO?.Complete();

if (_readBodyTask != null)
{
    await _readBodyTask;
}

was somehow skipped it could cause AVs. So adding a try {} finally {} to ensure that specific cleanup code isn't skipped.

Copilot AI review requested due to automatic review settings May 7, 2026 21:55
@BrennanConroy BrennanConroy added the feature-iis Includes: IIS, ANCM label May 7, 2026
@BrennanConroy BrennanConroy requested a review from halter73 as a code owner May 7, 2026 21:55
@BrennanConroy BrennanConroy added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label May 7, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens IIS in-process request teardown by ensuring critical IO/pipeline completion logic runs even if an exception is thrown during late-stage request/response processing, reducing the risk of native callbacks interacting with already-collected managed state.

Changes:

  • Wraps late-stage response completion / stream stopping / end-of-request production in a try/finally so the pipe/AsyncIO completion sequence always runs.
  • Moves the “complete writer/reader + drain write/read tasks + AsyncIO.Complete” sequence into the finally block with additional explanatory comments.

Comment on lines +101 to +125
finally
{
// If the request was aborted and no response was sent, we use status code 499 for logging
StatusCode = ClientDisconnected ? StatusCodes.Status499ClientClosedRequest : 0;
success = false;
}
// Defensive finally in case any of the above code throws an exception

// Complete response writer and request reader pipe sides
_bodyOutput.Complete();
_bodyInputPipe?.Reader.Complete();
// Important cleanup, this ensures native is done with async completions and we can cleanup resources
// If this somehow didn't run and native was still doing async completions, we could end up AVing
// when trying to access managed objects that were already collected.

// Allow writes to drain
if (_writeBodyTask != null)
{
await _writeBodyTask;
}
// Complete response writer and request reader pipe sides
_bodyOutput.Complete();
_bodyInputPipe?.Reader.Complete();

// Cancel all remaining IO, there might be reads pending if not entire request body was sent by client
AsyncIO?.Complete();
// Allow writes to drain
if (_writeBodyTask != null)
{
await _writeBodyTask;
}

if (_readBodyTask != null)
{
await _readBodyTask;
// Cancel all remaining IO, there might be reads pending if not entire request body was sent by client
AsyncIO?.Complete();

if (_readBodyTask != null)
{
await _readBodyTask;
}
Comment on lines +105 to +107
// Important cleanup, this ensures native is done with async completions and we can cleanup resources
// If this somehow didn't run and native was still doing async completions, we could end up AVing
// when trying to access managed objects that were already collected.
@BrennanConroy BrennanConroy merged commit b9c48b3 into main May 8, 2026
41 of 43 checks passed
@BrennanConroy BrennanConroy deleted the brecon/iisrequestcleanup branch May 8, 2026 00:18
@BrennanConroy BrennanConroy added this to the 11.0-preview5 milestone May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-iis Includes: IIS, ANCM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants