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

refactor(cli): add async version of resolve_addr function #8743

Merged
merged 2 commits into from Dec 15, 2020

Conversation

magurotuna
Copy link
Member

While I'm looking into #7824, I found to_socket_addrs, which is a blocking API, was called in resolve_addr function.
So this commit adds a new function that is an asynchronous version of resolve_addr using tokio::net::lookup_host.
And accordingly, renames the synchronous version to resolve_addr_sync.
This allows async ops to resolve hosts with non-blocking.

This commit adds a new function that is an asynchronous version of
`resolve_addr` using `tokio::net::lookup_host`, and accordingly, renames
the synchronous version to `resolve_addr_sync`.
This allows async ops to resolve hosts with non-blocking.
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

Nice work. Thanks!

It would be cool to expose these as standalone ops!

I am concerned that the asynchronous version does not go through the system's getaddrinfo() and potentially ignores things like /etc/hosts. Does tokio::net::lookup_host call getaddrinfo() in a thread pool (with spawn_blocking) or is it using a separate asynchronous DNS stack?

@magurotuna
Copy link
Member Author

@ry

It would be cool to expose these as standalone ops!

Sure, I'll do that.

I am concerned that the asynchronous version does not go through the system's getaddrinfo() and potentially ignores things like /etc/hosts. Does tokio::net::lookup_host call getaddrinfo() in a thread pool (with spawn_blocking) or is it using a separate asynchronous DNS stack?

Good point. Looking at tokio's implementation, it eventually calls std::net::ToSocketAddrs with spawn_blocking.
Refs:

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - thanks!

@ry ry merged commit 6356345 into denoland:master Dec 15, 2020
@magurotuna magurotuna deleted the refactor-resolve-addr branch December 15, 2020 12:39
kitsonk pushed a commit to lucacasonato/deno that referenced this pull request Dec 18, 2020
This commit adds a new function that is an asynchronous version of
`resolve_addr` using `tokio::net::lookup_host`, and accordingly, renames
the synchronous version to `resolve_addr_sync`.
This allows async ops to resolve hosts with non-blocking.
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.

None yet

2 participants