Skip to content

Conversation

@BrennanConroy
Copy link
Member

@BrennanConroy BrennanConroy commented Feb 10, 2026

Avoid disposing invalid handle in HttpSys server close

Description

HttpSys docs say we should close the request queue with HttpCloseRequestQueue and not with CloseHandle. So the PR #61325 noticed this while making an unrelated change and updated the code to call HttpCloseRequestQueue. Unfortunately, we kept the old Handle.Dispose() code which is effectively a double free.

The fix is to mark the handle as invalid after closing the request queue so we avoid the double free.

Fixes #65259

Customer Impact

Error log (and exception from Dispose) when cleaning up HttpSys server.
Issue: #65259

Regression?

  • Yes
  • No

Regressed in #61325 (10.0-p5)

Risk

  • High
  • Medium
  • Low

Customer reported exception was easy to repro and thus easy to verify it was fixed with the change.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@BrennanConroy BrennanConroy added this to the 10.0.x milestone Feb 10, 2026
Copilot AI review requested due to automatic review settings February 10, 2026 02:31
@BrennanConroy BrennanConroy added Servicing-consider Shiproom approval is required for the issue area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions labels Feb 10, 2026
Copy link
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

Fixes a regression in the HttpSys server shutdown path by preventing a double-close of the request queue handle after switching to HttpCloseRequestQueue, aligning handle lifecycle management with Http.Sys API requirements.

Changes:

  • Keep HttpCloseRequestQueue(Handle) as the authoritative close operation for the request queue.
  • Prevent a subsequent SafeHandle.Dispose() from attempting to close the same native handle by marking the handle invalid after closing.
  • Add clarifying comments about disposal order relative to ThreadPoolBoundHandle.


BoundHandle.Dispose();
Handle.Dispose();
Handle.SetHandleAsInvalid();
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

After Handle.SetHandleAsInvalid(), consider still calling Handle.Dispose() to suppress the SafeHandle finalizer and release managed resources; disposal should be a no-op for the native handle once it’s marked invalid.

Suggested change
Handle.SetHandleAsInvalid();
Handle.SetHandleAsInvalid();
Handle.Dispose();

Copilot uses AI. Check for mistakes.
@BrennanConroy BrennanConroy added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Feb 10, 2026
@dotnet-policy-service
Copy link
Contributor

Hi @@BrennanConroy. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

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 Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants