Skip to content

[release/8.0-staging] [DNS] Ignore ObjectDisposedException on CancellationToken Callback #115841

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

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented May 21, 2025

Backport of #115765 to release/8.0-staging

/cc @liveans

Customer Impact

Reported by 2 customers: Customer application crashes due to unhandled ObjectDisposedException when they call DNS APIs (e.g. Dns.GetHostAddress) and then start cancellation of the request (e.g. when the DNS response takes long time due to network environment).

Technical details: There’s a race condition between the DNS request completion and the CancellationToken-registered callback in our code.
We're currently checking whether the DNS request has completed before initiating the cancellation process, but it's possible for the DNS request to complete right after the check. That can lead to the ObjectDisposedException, because we assume the DNS request is still valid, while it was completed in the meantime.

Regression

Yes, introduced in #63904 (part of .NET 7.0).

Testing

Verified on targeted stress test - without the change in fails 3x in 24h. With the fix 30 h & ~70h without a crash.

Risk

Low, fix is targeted to a specific case.

Package authoring no longer needed in .NET 9

N/A - this is not a NuGet package, just part of runtime itself

@liveans liveans added the Servicing-consider Issue for next servicing release review label May 21, 2025
@karelz karelz added this to the 8.0.x milestone May 22, 2025
@karelz karelz removed the Servicing-consider Issue for next servicing release review label May 22, 2025
Copy link
Member

@karelz karelz left a comment

Choose a reason for hiding this comment

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

I'd add comment to the @this.DangerousAddRef call that it may throw and it is OK as no-op

Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@karelz karelz added the Servicing-consider Issue for next servicing release review label May 26, 2025
@liveans
Copy link
Member

liveans commented May 27, 2025

Approved by @SteveMCarroll via email on 05/27.

@liveans liveans added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels May 27, 2025
@liveans liveans merged commit 5b4a588 into release/8.0-staging May 27, 2025
123 of 131 checks passed
@jkotas jkotas deleted the backport/pr-115765-to-release/8.0-staging branch May 31, 2025 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants