Skip to content

Expand Windows DNS sync-query detection for inline-completing names - #130411

Merged
rzikm merged 5 commits into
mainfrom
copilot/dotnetruntime-130374-fix-dns-resolver-memory-leak
Jul 14, 2026
Merged

Expand Windows DNS sync-query detection for inline-completing names#130411
rzikm merged 5 commits into
mainfrom
copilot/dotnetruntime-130374-fix-dns-resolver-memory-leak

Conversation

Copilot AI commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This broadens the Windows DnsResolver workaround for pre-22000 DnsQueryEx inline completion so it covers the full set of names the OS team identified as synchronously completing, not just localhost. The change keeps the existing async path for all other names and avoids the extra DNS_QUERY_NO_WIRE_QUERY probe.

  • Behavior

    • Route these names directly through the synchronous path on affected Windows versions:
      • IP literals
      • machine hostname
      • loopback
      • localhost
      • ..DnsServers
      • ..localmachine
  • Implementation

    • Replace the localhost-only check with a broader predicate for synchronously completing names.
    • Use IPAddress.IsValid(...) for allocation-free IP literal detection.
    • Compare against the current hostname without process-wide caching.
    • If hostname lookup fails, fall back to the normal async path rather than introducing a new failure mode.
  • Regression coverage

    • Expand the existing Windows regression test to cover the added synchronous-completion cases through both sync and async entry points.
if (IPAddress.IsValid(name))
{
    return true;
}

Copilot AI self-assigned this Jul 9, 2026
Copilot AI review requested due to automatic review settings July 9, 2026 14:20
Copilot AI removed the request for review from Copilot July 9, 2026 14:20
Copilot AI linked an issue Jul 9, 2026 that may be closed by this pull request
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 9, 2026 14:54
Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 9, 2026 15:08
Copilot AI changed the title [WIP] Fix memory leak in DnsResolver for Windows 10/Server 2019 Expand Windows DNS sync-query detection for inline-completing names Jul 9, 2026
Copilot finished work on behalf of rzikm July 9, 2026 15:10
Copilot AI requested a review from rzikm July 9, 2026 15:10
@rzikm
rzikm marked this pull request as ready for review July 9, 2026 15:22
Copilot AI review requested due to automatic review settings July 9, 2026 15:22
Copilot AI had a problem deploying to copilot-pat-pool July 9, 2026 15:22 Failure

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the Windows System.Net.NameResolution DnsQueryEx workaround by expanding the set of query names treated as “known to complete synchronously” on affected Windows versions, and extends the existing regression test to exercise those names through both sync/async resolver entry points.

Changes:

  • Replace the localhost-only fast-path with IsSynchronouslyCompletingQueryName, including IP literal detection and additional special-case names.
  • Route those names directly through the synchronous DnsQueryEx path when running async on pre-22000 Windows versions.
  • Expand the Windows regression test to cover the additional “synchronously completing” names.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Broaden the pre-22000 async inline-completion workaround to a wider set of names via a new predicate.
src/libraries/System.Net.NameResolution/tests/FunctionalTests/DnsResolverTest.cs Expand the existing Windows regression test to cover the broader set of synchronously-completing names.

Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/tests/FunctionalTests/DnsResolverTest.cs Outdated

@MichalPetryka MichalPetryka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add an assert that we never got null and a memory leak here?

Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
…n true on hostname failure, fix test data

Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 13, 2026 10:07
Copilot finished work on behalf of rzikm July 13, 2026 10:08
Copilot AI requested a review from rzikm July 13, 2026 10:08
…tion test

Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 13, 2026 10:21
Copilot finished work on behalf of rzikm July 13, 2026 10:22
Copilot finished work on behalf of rzikm July 13, 2026 13:21
@rzikm
rzikm requested a review from a team July 13, 2026 14:13
@rzikm

rzikm commented Jul 13, 2026

Copy link
Copy Markdown
Member

/ba-g System.Text.Json failures are unrelated

@rzikm
rzikm merged commit 46178ac into main Jul 14, 2026
78 of 82 checks passed
@rzikm
rzikm deleted the copilot/dotnetruntime-130374-fix-dns-resolver-memory-leak branch July 14, 2026 10:35
eiriktsarpalis pushed a commit that referenced this pull request Jul 15, 2026
…130411)

<!-- -->

This broadens the Windows `DnsResolver` workaround for pre-22000
`DnsQueryEx` inline completion so it covers the full set of names the OS
team identified as synchronously completing, not just `localhost`. The
change keeps the existing async path for all other names and avoids the
extra `DNS_QUERY_NO_WIRE_QUERY` probe.

- **Behavior**
- Route these names directly through the synchronous path on affected
Windows versions:
    - IP literals
    - machine hostname
    - `loopback`
    - `localhost`
    - `..DnsServers`
    - `..localmachine`

- **Implementation**
- Replace the `localhost`-only check with a broader predicate for
synchronously completing names.
- Use `IPAddress.IsValid(...)` for allocation-free IP literal detection.
  - Compare against the current hostname without process-wide caching.
- If hostname lookup fails, fall back to the normal async path rather
than introducing a new failure mode.

- **Regression coverage**
- Expand the existing Windows regression test to cover the added
synchronous-completion cases through both sync and async entry points.

```csharp
if (IPAddress.IsValid(name))
{
    return true;
}
```

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview7 milestone Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

DnsResolver Windows 10/Server 2019 memory leak

5 participants