Avoid disposing invalid handle in HttpSys server close#65378
Avoid disposing invalid handle in HttpSys server close#65378BrennanConroy wants to merge 1 commit intorelease/10.0from
Conversation
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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.
| Handle.SetHandleAsInvalid(); | |
| Handle.SetHandleAsInvalid(); | |
| Handle.Dispose(); |
|
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. |
Avoid disposing invalid handle in HttpSys server close
Description
HttpSys docs say we should close the request queue with
HttpCloseRequestQueueand not withCloseHandle. So the PR #61325 noticed this while making an unrelated change and updated the code to callHttpCloseRequestQueue. Unfortunately, we kept the oldHandle.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?
Regressed in #61325 (10.0-p5)
Risk
Customer reported exception was easy to repro and thus easy to verify it was fixed with the change.
Verification
Packaging changes reviewed?