fix: avoid 20s DNS timeout blocking UDP packet connections#239
Merged
myleshorton merged 3 commits intomainfrom Apr 15, 2026
Merged
fix: avoid 20s DNS timeout blocking UDP packet connections#239myleshorton merged 3 commits intomainfrom
myleshorton merged 3 commits intomainfrom
Conversation
…0s DNS timeout sendInfo() for UDP packet connections was calling net.ResolveUDPAddr() to resolve the destination hostname synchronously. In fakeip mode, the system DNS points at the TUN address (10.10.1.2:53), so the resolution re-enters sing-box and stalls for 20 seconds before timing out. This blocks the entire packet connection — no data flows until the timeout completes, then the connection is killed. The routing pipeline already resolves destinations into metadata.DestinationAddresses earlier in the flow. Use those instead of re-resolving. Diagnosed from Freshdesk #172590 (internal user reporting slow/inconsistent connection). Every new UDP/QUIC connection was blocked for 20s by this path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… the timeout Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes UDP/QUIC packet connections stalling for ~20 seconds in fakeip mode by removing a synchronous DNS resolution step (net.ResolveUDPAddr) from the client-context injection path and instead using addresses already resolved by the routing pipeline (metadata.DestinationAddresses).
Changes:
- Update UDP
sendInfo()to useInboundContext.DestinationAddresses[0]for domain destinations instead of callingnet.ResolveUDPAddr. - Add/adjust tests to cover: IP destinations, domain destinations with pre-resolved addresses, and domain destinations without resolved addresses (error case).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tracker/clientcontext/injector.go |
Avoids synchronous DNS resolution for UDP destinations by using DestinationAddresses and failing fast when not available. |
tracker/clientcontext/injector_test.go |
Updates/adds unit tests to validate the new resolved-address behavior and the new error path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
garmr-ulfr
approved these changes
Apr 14, 2026
Collaborator
garmr-ulfr
left a comment
garmr-ulfr
left a comment
There was a problem hiding this comment.
LGTM. Just one comment. I'm not sure if Copilot's comment is correct or not, but it really depends on whether we want to worry about IPv6. 🤷
… is empty Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sendInfo()for UDP packet connections was callingnet.ResolveUDPAddr()to resolve the destination hostname on every new connectionmetadata.DestinationAddresses— this second lookup is redundantTest plan
TestSendInfoWithIPDestination— IP destination still worksTestSendInfoWithDomainAndResolvedAddresses— domain with pre-resolved addresses uses them directlyTestSendInfoWithDomainNoResolvedAddresses— domain without resolved addresses returns clear errorDiagnosed from Freshdesk #172590.
🤖 Generated with Claude Code