hostByName timeout fixes (#8787)

* single impl

cores/esp8266/IPAddress.cpp

@@ -27,6 +27,11 @@ IPAddress::IPAddress(const IPAddress& from)
     ip_addr_copy(_ip, from._ip);
 }
 
+IPAddress::IPAddress(IPAddress&& from)
+{
+    ip_addr_copy(_ip, from._ip);
+}
+
 IPAddress::IPAddress() {
     _ip = *IP_ANY_TYPE; // lwIP's v4-or-v6 generic address
 }

cores/esp8266/IPAddress.h

@@ -66,7 +66,9 @@ class IPAddress: public Printable {
     public:
         // Constructors
         IPAddress();
-        IPAddress(const IPAddress& from);
+        IPAddress(const IPAddress&);
+        IPAddress(IPAddress&&);
+
         IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet);
         IPAddress(uint32_t address) { ctor32(address); }
         IPAddress(unsigned long address) { ctor32(address); }

libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp

@@ -22,8 +22,11 @@
 
  */
 
+#include <cstring>
 #include <list>
-#include <string.h>
+#include <memory>
+#include <type_traits>
+
 #include <coredecls.h>
 #include <PolledTimeout.h>
 #include "ESP8266WiFi.h"
@@ -595,9 +598,83 @@ bool ESP8266WiFiGenericClass::isSleepLevelMax () {
 // ------------------------------------------------ Generic Network function ---------------------------------------------
 // -----------------------------------------------------------------------------------------------------------------------
 
-void wifi_dns_found_callback(const char *name, const ip_addr_t *ipaddr, void *callback_arg);
+namespace {
 
-static bool _dns_lookup_pending = false;
+struct _dns_found_result {
+    IPAddress addr;
+    bool done;
+};
+
+}
+
+static void _dns_found_callback(const char *, const ip_addr_t *, void *);
+
+static int hostByNameImpl(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType) {
+    if (aResult.fromString(aHostname)) {
+        DEBUG_WIFI_GENERIC("[hostByName] Host: %s is IP!\n", aHostname);
+        return 1;
+    }
+
+    static_assert(std::is_same_v<uint8_t, std::underlying_type_t<decltype(resolveType)>>, "");
+    DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname);
+
+    ip_addr_t addr;
+    auto pending = std::make_unique<_dns_found_result>(
+        _dns_found_result{
+            .addr = IPADDR_NONE,
+            .done = false,
+        });
+
+    err_t err = dns_gethostbyname_addrtype(aHostname,
+        &addr, &_dns_found_callback, pending.get(),
+        static_cast<uint8_t>(resolveType));
+
+    switch (err) {
+    // Address already known
+    case ERR_OK:
+        aResult = addr;
+        break;
+
+    // We are no longer able to issue requests
+    case ERR_MEM:
+        break;
+
+    // We need to wait for c/b to fire *or* we exit on our own timeout
+    // (which also requires us to notify the c/b that it is supposed to delete the pending obj)
+    case ERR_INPROGRESS:
+        // Re-check every 10ms, we expect this to happen fast
+        esp_delay(timeout_ms,
+            [&]() {
+                return !pending->done;
+            }, 10);
+
+        if (pending->done) {
+            if ((pending->addr).isSet()) {
+                aResult = pending->addr;
+                err = ERR_OK;
+            }
+        } else {
+            pending->done = true;
+            pending.release();
+            err = ERR_TIMEOUT;
+        }
+
+        break;
+    }
+
+    if (err == ERR_OK) {
+        DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str());
+        return 1;
+    }
+
+    DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %s (%d)!\n",
+            aHostname,
+            (err == ERR_TIMEOUT) ? "Timeout" :
+            (err == ERR_INPROGRESS) ? "No response" :
+                "Unknown", static_cast<int>(err));
+
+    return 0;
+}
 
 /**
  * Resolve the given hostname to an IP address.
@@ -608,99 +685,18 @@ static bool _dns_lookup_pending = false;
  */
 int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult)
 {
-    return hostByName(aHostname, aResult, 10000);
+    return hostByNameImpl(aHostname, aResult, DNSDefaultTimeoutMs, DNSResolveTypeDefault);
 }
 
-
 int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms)
 {
-    ip_addr_t addr;
+    return hostByNameImpl(aHostname, aResult, timeout_ms, DNSResolveTypeDefault);
-    aResult = static_cast<uint32_t>(INADDR_NONE);
-
-    if(aResult.fromString(aHostname)) {
-        // Host name is a IP address use it!
-        DEBUG_WIFI_GENERIC("[hostByName] Host: %s is a IP!\n", aHostname);
-        return 1;
-    }
-
-    DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname);
-#if LWIP_IPV4 && LWIP_IPV6
-    err_t err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult,LWIP_DNS_ADDRTYPE_DEFAULT);
-#else
-    err_t err = dns_gethostbyname(aHostname, &addr, &wifi_dns_found_callback, &aResult);
-#endif
-    if(err == ERR_OK) {
-        aResult = IPAddress(&addr);
-    } else if(err == ERR_INPROGRESS) {
-        _dns_lookup_pending = true;
-        // Will resume on timeout or when wifi_dns_found_callback fires.
-        // The final argument, intvl_ms, to esp_delay influences how frequently
-        // the scheduled recurrent functions (Schedule.h) are probed; here, to allow
-        // the ethernet driver perform work.
-        esp_delay(timeout_ms, []() { return _dns_lookup_pending; }, 1);
-        _dns_lookup_pending = false;
-        if(aResult.isSet()) {
-            err = ERR_OK;
-        }
-    }
-
-    if(err == ERR_OK) {
-        DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str());
-        return 1;
-    }
-    
-    DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %s (%d)!\n", aHostname, lwip_strerr(err), (int)err);
-    return 0;
 }
 
 #if LWIP_IPV4 && LWIP_IPV6
 int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResult, uint32_t timeout_ms, DNSResolveType resolveType)
 {
-    ip_addr_t addr;
+    return hostByNameImpl(aHostname, aResult, timeout_ms, resolveType);
-    err_t err;
-    aResult = static_cast<uint32_t>(INADDR_NONE);
-
-    if(aResult.fromString(aHostname)) {
-        // Host name is a IP address use it!
-        DEBUG_WIFI_GENERIC("[hostByName] Host: %s is a IP!\n", aHostname);
-        return 1;
-    }
-
-    DEBUG_WIFI_GENERIC("[hostByName] request IP for: %s\n", aHostname);
-    switch(resolveType)
-    {
-      // Use selected addrtype
-      case DNSResolveType::DNS_AddrType_IPv4:
-      case DNSResolveType::DNS_AddrType_IPv6:
-      case DNSResolveType::DNS_AddrType_IPv4_IPv6:
-      case DNSResolveType::DNS_AddrType_IPv6_IPv4:
-         err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult, (uint8_t) resolveType);
-	 break;
-      default:
-         err = dns_gethostbyname_addrtype(aHostname, &addr, &wifi_dns_found_callback, &aResult, LWIP_DNS_ADDRTYPE_DEFAULT); // If illegal type, use default.
-	 break;
-    }
-
-    if(err == ERR_OK) {
-        aResult = IPAddress(&addr);
-    } else if(err == ERR_INPROGRESS) {
-        _dns_lookup_pending = true;
-        // will resume on timeout or when wifi_dns_found_callback fires
-        esp_delay(timeout_ms, []() { return _dns_lookup_pending; });
-        _dns_lookup_pending = false;
-        // will return here when dns_found_callback fires
-        if(aResult.isSet()) {
-            err = ERR_OK;
-        }
-    }
-
-    if(err == ERR_OK) {
-        DEBUG_WIFI_GENERIC("[hostByName] Host: %s IP: %s\n", aHostname, aResult.toString().c_str());
-        return 1;
-    }
-
-    DEBUG_WIFI_GENERIC("[hostByName] Host: %s lookup error: %d!\n", aHostname, (int)err);
-    return 0;
 }
 #endif
 
@@ -710,16 +706,19 @@ int ESP8266WiFiGenericClass::hostByName(const char* aHostname, IPAddress& aResul
  * @param ipaddr
  * @param callback_arg
  */
-void wifi_dns_found_callback(const char *name, const ip_addr_t *ipaddr, void *callback_arg)
+static void _dns_found_callback(const char*, const ip_addr_t* ipaddr, void* arg)
 {
-    (void) name;
+    auto result = reinterpret_cast<_dns_found_result*>(arg);
-    if (!_dns_lookup_pending) {
+    if (result->done) {
+        delete result;
         return;
     }
-    if(ipaddr) {
+
-        (*reinterpret_cast<IPAddress*>(callback_arg)) = IPAddress(ipaddr);
+    if (ipaddr) {
+        result->addr = IPAddress(ipaddr);
     }
-    _dns_lookup_pending = false; // resume hostByName
+
+    result->done = true;
     esp_schedule();
 }
 

libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.h

@@ -24,6 +24,10 @@
 #define ESP8266WIFIGENERIC_H_
 
 #include "ESP8266WiFiType.h"
+
+#include <IPAddress.h>
+#include <lwip/dns.h>
+
 #include <functional>
 #include <memory>
 
@@ -44,12 +48,15 @@ typedef void (*WiFiEventCb)(WiFiEvent_t);
 
 enum class DNSResolveType: uint8_t
 {
-    DNS_AddrType_IPv4 = 0,	// LWIP_DNS_ADDRTYPE_IPV4 = 0
+    DNS_AddrType_IPv4 = LWIP_DNS_ADDRTYPE_IPV4,
-    DNS_AddrType_IPv6,		// LWIP_DNS_ADDRTYPE_IPV6 = 1
+    DNS_AddrType_IPv6 = LWIP_DNS_ADDRTYPE_IPV6,
-    DNS_AddrType_IPv4_IPv6,	// LWIP_DNS_ADDRTYPE_IPV4_IPV6 = 2
+    DNS_AddrType_IPv4_IPv6 = LWIP_DNS_ADDRTYPE_IPV4_IPV6,
-    DNS_AddrType_IPv6_IPv4	// LWIP_DNS_ADDRTYPE_IPV6_IPV4 = 3
+    DNS_AddrType_IPv6_IPv4 = LWIP_DNS_ADDRTYPE_IPV6_IPV4,
 };
 
+inline constexpr auto DNSDefaultTimeoutMs = 10000;
+inline constexpr auto DNSResolveTypeDefault = static_cast<DNSResolveType>(LWIP_DNS_ADDRTYPE_DEFAULT);
+
 struct WiFiState;
 
 class ESP8266WiFiGenericClass {

tests/host/common/ClientContextTools.cpp

@@ -37,18 +37,27 @@
 
 #include <netdb.h>  // gethostbyname
 
-err_t dns_gethostbyname(const char* hostname, ip_addr_t* addr, dns_found_callback found,
+err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback, void*,
-                        void* callback_arg)
+                                 u8 type)
 {
-    (void)callback_arg;
+    auto* hbn = gethostbyname(hostname);
-    (void)found;
-    struct hostent* hbn = gethostbyname(hostname);
     if (!hbn)
         return ERR_TIMEOUT;
-    addr->addr = *(uint32_t*)hbn->h_addr_list[0];
+
+    uint32_t tmp;
+    std::memcpy(&tmp, hbn->h_addr_list[0], sizeof(tmp));
+    addr->addr = tmp;
+
     return ERR_OK;
 }
 
+err_t dns_gethostbyname_addrtype(const char* hostname, ip_addr_t* addr, dns_found_callback found,
+                                 void* callback_arg)
+{
+    return dns_gethostbyname_addrtype(hostname, addr, found, callback_arg,
+                                      LWIP_DNS_ADDRTYPE_DEFAULT);
+}
+
 static struct tcp_pcb mock_tcp_pcb;
 tcp_pcb*              tcp_new(void)
 {