From 3dc865ea8861bea174dc880ed209f37014c1e972 Mon Sep 17 00:00:00 2001
From: Martin Algesten <martin@algesten.se>
Date: Mon, 8 Jul 2024 09:04:07 +0200
Subject: [PATCH] DefaultResolver only lookup async when needed

---
 src/resolver.rs | 81 ++++++++++++++++++++++++++++++++++---------------
 src/time.rs     | 10 ++++++
 2 files changed, 67 insertions(+), 24 deletions(-)

diff --git a/src/resolver.rs b/src/resolver.rs
index 396831bb..5b7f5057 100644
--- a/src/resolver.rs
+++ b/src/resolver.rs
@@ -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 {
@@ -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);
@@ -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)),
+            // 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,
diff --git a/src/time.rs b/src/time.rs
index b900db0d..89f0481f 100644
--- a/src/time.rs
+++ b/src/time.rs
@@ -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::*;