Skip to content

fix: make domain name resolution async - #1642

Open
citizen-stig wants to merge 3 commits into
paritytech:masterfrom
citizen-stig:nikolai/async-name-resonlution
Open

fix: make domain name resolution async#1642
citizen-stig wants to merge 3 commits into
paritytech:masterfrom
citizen-stig:nikolai/async-name-resonlution

Conversation

@citizen-stig

@citizen-stig citizen-stig commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

So faulty resovler won't starve runtime.

try_connect_over_tcp resolved DNS with uri.socket_addrs(|| None) (initial connect and redirect) → std::net::ToSocketAddrs → blocking getaddrinfo on the current Tokio worker thread, outside the timeout. The connection_timeout only ever wrapped TcpStream::connect (inside connect() via tokio::select!).

Include resolution into timeout

@citizen-stig citizen-stig changed the title make domain name resolution async Make domain name resolution async Jun 10, 2026
@citizen-stig

Copy link
Copy Markdown
Contributor Author

Hey @lexnv , if you can take a look on this PR too, that would be fantastic. Failures look the same as in #1643 . Formatting is fixed.

@citizen-stig citizen-stig changed the title Make domain name resolution async fix: make domain name resolution async Jun 12, 2026
@citizen-stig
citizen-stig force-pushed the nikolai/async-name-resonlution branch from a138a08 to ccacde4 Compare July 30, 2026 07:12
@citizen-stig

Copy link
Copy Markdown
Contributor Author

@DenzelPenzel I also would like to bump this PR

// internally, so IP literals and default ports behave exactly as before.
match uri.host() {
Some(url::Host::Domain(domain)) => {
tokio::net::lookup_host((domain, port)).await.map(|addrs| addrs.collect()).map_err(|e| {

@DenzelPenzel DenzelPenzel Jul 30, 2026

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.

timeout stops waiting for lookup_host, but it does not cancel the underlying spawn_blocking DNS lookup. wdty?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's true, but I don't know how it can addressed.
But current approach makes situation a little better than on main, because on main broken resolver will stall whole runtime, while here we will have task sitting on blocking pool thread.

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.

3 participants