Skip to content

Commit

Permalink
DefaultResolver only lookup async when needed
Browse files Browse the repository at this point in the history
  • Loading branch information
algesten committed Jul 8, 2024
1 parent 8886d80 commit 3dc865e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 24 deletions.
81 changes: 57 additions & 24 deletions src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ use std::net::{SocketAddr, ToSocketAddrs};
use std::sync::mpsc::{self, RecvTimeoutError};
use std::thread::{self};
use std::time::Duration;
use std::vec::IntoIter;

use http::uri::{Authority, Scheme};
use http::Uri;

use crate::error::TimeoutReason;
use crate::time::DurationExt;
use crate::Error;

pub trait Resolver: fmt::Debug + 'static {
Expand All @@ -30,33 +33,45 @@ pub enum AddrSelect {
First,
// TODO(martin): implement round robin per hostname
}
impl DefaultResolver {
pub fn host_and_port(scheme: &Scheme, authority: &Authority) -> String {
let port = authority
.port_u16()
.unwrap_or_else(|| DefaultResolver::scheme_default_port(scheme));

format!("{}:{}", authority.host(), port)
}

pub fn scheme_default_port(scheme: &Scheme) -> u16 {
if *scheme == Scheme::HTTP {
80
} else if *scheme == Scheme::HTTPS {
443
} else {
// Unclear why http-crate Scheme is not an enum. Are we expecting
// more schemes in the future?
unreachable!("Unknown scheme")
}
}
}

impl Resolver for DefaultResolver {
fn resolve(&self, uri: &Uri, timeout: Duration) -> Result<SocketAddr, Error> {
let host = uri
.authority()
.map(|a| a.host())
.ok_or(Error::Other("No host in uri"))?
// There is no way around allocating here. We can't use a scoped thread below,
// because if we time out, we're going to exit the function and leave the
// thread running (causing a panic as per scoped thread contract).
.to_string();

// TODO(martin): On Linux we have getaddrinfo_a which is a libc async way of
// doing host lookup. We should make a subcrate that uses a native async method
// when possible, and otherwise fall back on this thread behavior.
let (tx, rx) = mpsc::sync_channel(1);
thread::spawn(move || tx.send(host.to_socket_addrs()).ok());

let iter = match rx.recv_timeout(timeout) {
Ok(v) => v,
Err(c) => match c {
// Timeout results in None
RecvTimeoutError::Timeout => return Err(Error::Timeout(TimeoutReason::Resolver)),
// The sender going away is nonsensical. Did the thread just die?
RecvTimeoutError::Disconnected => unreachable!("mpsc sender gone"),
},
}?;
let scheme = uri.scheme().ok_or(Error::Other("No scheme in uri"))?;
let authority = uri.authority().ok_or(Error::Other("No host in uri"))?;

// This will be on the form "myspecialhost.org:1234". The port is mandatory.
let addr = DefaultResolver::host_and_port(scheme, authority);

// Determine if we want to use the async behavior.
let use_sync = timeout.is_not_happening();

let iter = if use_sync {
// When timeout is not set, we do not spawn any threads.
addr.to_socket_addrs()?
} else {
resolve_async(addr, timeout)?
};

let wanted = self.family.keep_wanted(iter);
let maybe_addr = self.select.choose(wanted);
Expand All @@ -65,6 +80,24 @@ impl Resolver for DefaultResolver {
}
}

fn resolve_async(addr: String, timeout: Duration) -> Result<IntoIter<SocketAddr>, Error> {
// TODO(martin): On Linux we have getaddrinfo_a which is a libc async way of
// doing host lookup. We should make a subcrate that uses a native async method
// when possible, and otherwise fall back on this thread behavior.
let (tx, rx) = mpsc::sync_channel(1);
thread::spawn(move || tx.send(addr.to_socket_addrs()).ok());

match rx.recv_timeout(timeout) {
Ok(v) => Ok(v?),
Err(c) => match c {
// Timeout results in None
RecvTimeoutError::Timeout => return Err(Error::Timeout(TimeoutReason::Resolver)),

Check failure on line 94 in src/resolver.rs

View workflow job for this annotation

GitHub Actions / Lint

unneeded `return` statement

Check failure on line 94 in src/resolver.rs

View workflow job for this annotation

GitHub Actions / Lint

unneeded `return` statement
// The sender going away is nonsensical. Did the thread just die?
RecvTimeoutError::Disconnected => unreachable!("mpsc sender gone"),
},
}
}

impl IpFamily {
pub fn keep_wanted<'a>(
&'a self,
Expand Down
10 changes: 10 additions & 0 deletions src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ impl Ord for Instant {
}
}

pub trait DurationExt {
fn is_not_happening(&self) -> bool;
}

impl DurationExt for Duration {
fn is_not_happening(&self) -> bool {
*self >= Duration::from_secs(u64::MAX)
}
}

#[cfg(test)]
mod test {
use super::*;
Expand Down

0 comments on commit 3dc865e

Please sign in to comment.