Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some improvements to HttpSysDelegator logging and retry logic #2671

Merged

Conversation

NGloreous
Copy link
Contributor

This PR has two changes:

  1. Added logic to retry delegation when an ObjectDisposedException happens when delegating

    Update HttpSysDelegator to support detaching from and re-initializing queues when the http.sys queue no longer exists #2426 added support to detach from an existing queue when ERROR_OBJECT_NO_LONGER_EXISTS happens due to the owning process detaching from the queue. This works but there exists a race condition where other requests might have already obtained the old handle before we re-initialized and then they get an ObjectDisposedException when trying to use the old handle. Now we retry this error so the request will obtain the new handle and retry the delegation.

  2. Added an ID to the DelegationQueueState (which holds the current handle to the queue) and included that ID in most existing logs.

    We've seen some odd behavior with IIS and queue management around iisreset. With multiple threads trying to delegate, detach, re-initialize, it's hard to fully follow from logs what exactly is happening. This change uses the current activity's span id as the DelegationQueueState's id. This allows us to track which requests cause queues to be re-initialized/reset.

Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

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

This seems reasonable, thank you.

src/ReverseProxy/Delegation/HttpSysDelegator.cs Outdated Show resolved Hide resolved
src/ReverseProxy/Delegation/HttpSysDelegator.cs Outdated Show resolved Hide resolved
@NGloreous NGloreous closed this Dec 5, 2024
@NGloreous NGloreous reopened this Dec 5, 2024
@MihaZupan MihaZupan merged commit afab63c into microsoft:main Dec 5, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants