Synchronous DNS resolution can wedge a crawl past --max-time

man/httrack.1

@@ -3,7 +3,7 @@
 .\"
 .\" This file is generated by man/makeman.sh; do not edit by hand.
 .\" SPDX-License-Identifier: GPL-3.0-or-later
-.TH httrack 1 "14 July 2026" "httrack website copier"
+.TH httrack 1 "16 July 2026" "httrack website copier"
 .SH NAME
 httrack \- offline browser : copy websites to a local directory
 .SH SYNOPSIS
@@ -165,7 +165,7 @@ pause transfer if N bytes reached, and wait until lock file is deleted (\-\-max\
 .IP \-cN
 number of multiple connections (*c8) (\-\-sockets[=N])
 .IP \-TN
-timeout, number of seconds after a non\-responding link is shutdown (\-\-timeout[=N])
+timeout, number of seconds after a non\-responding link is shutdown; also bounds host name resolution (\-\-timeout[=N])
 .IP \-RN
 number of retries, in case of timeout or non\-fatal errors (*R1) (\-\-retries[=N])
 .IP \-JN

src/htsdns_selftest.c

@@ -63,9 +63,14 @@ typedef struct mock_host {
   int gai_err; /* non-zero: getaddrinfo returns this */
   int naddr;
   mock_addr addr[6];
-  int calls; /* times the backend resolved this host */
+  int calls;   /* times the backend resolved this host */
+  int slow_ms; /* non-zero: block this long, as a black-hole resolver would */
 } mock_host;
 
+/* Long enough to outlast the 1s --timeout the bounded resolve is checked
+   against, short enough to keep the self-test quick. */
+#define MOCK_SLOW_MS 3000
+
 static mock_host mock_hosts[] = {
     {"v4only.test", 0, 1, {{AF_INET, {1, 2, 3, 4}}}, 0},
     {"v6only.test", 0, 1, {{AF_INET6, {0x20, 0x01, 0x0d, 0xb8, [15] = 1}}}, 0},
@@ -95,8 +100,15 @@ static mock_host mock_hosts[] = {
       {AF_INET, {10, 0, 0, 6}}},
      0},
     {"nodns.test", EAI_NONAME, 0, {{0}}, 0},
+    /* resolves, but only well after --timeout: the #606 wedge */
+    {"slow.test", 0, 1, {{AF_INET, {127, 0, 0, 9}}}, 0, MOCK_SLOW_MS},
 };
 
+/* Serializes mock_host bookkeeping: a timed-out resolve is abandoned, so its
+   worker is still inside the backend while the test reads the counters. */
+static htsmutex mock_lock = HTSMUTEX_INIT;
+static int mock_finished = 0; /* backend calls that have returned */
+
 static mock_host *mock_find(const char *name) {
   for (size_t i = 0; i < sizeof(mock_hosts) / sizeof(mock_hosts[0]); i++) {
     if (strcmp(mock_hosts[i].name, name) == 0)
@@ -106,8 +118,34 @@ static mock_host *mock_find(const char *name) {
 }
 
 static void mock_reset_calls(void) {
+  hts_mutexlock(&mock_lock);
   for (size_t i = 0; i < sizeof(mock_hosts) / sizeof(mock_hosts[0]); i++)
     mock_hosts[i].calls = 0;
+  mock_finished = 0;
+  hts_mutexrelease(&mock_lock);
+}
+
+static int mock_read_calls(const char *name) {
+  int calls;
+
+  hts_mutexlock(&mock_lock);
+  calls = mock_find(name)->calls;
+  hts_mutexrelease(&mock_lock);
+  return calls;
+}
+
+/* Wait for n backend calls to return, ordering their writes against ours. */
+static void mock_wait_finished(int n) {
+  for (;;) {
+    hts_boolean done;
+
+    hts_mutexlock(&mock_lock);
+    done = (mock_finished >= n) ? HTS_TRUE : HTS_FALSE;
+    hts_mutexrelease(&mock_lock);
+    if (done)
+      break;
+    Sleep(10);
+  }
 }
 
 /* Build one addrinfo node owning its sockaddr (freed by mock_freeaddrinfo). */
@@ -133,10 +171,10 @@ static struct addrinfo *mock_mkai(const mock_addr *a) {
   return ai;
 }
 
-static int HTS_RESOLVER_CALL mock_getaddrinfo(const char *node,
-                                              const char *service,
-                                              const struct addrinfo *hints,
-                                              struct addrinfo **res) {
+static int HTS_RESOLVER_CALL mock_getaddrinfo_(const char *node,
+                                               const char *service,
+                                               const struct addrinfo *hints,
+                                               struct addrinfo **res) {
   mock_host *const h = mock_find(node);
   const int want = (hints != NULL) ? hints->ai_family : PF_UNSPEC;
   struct addrinfo *head = NULL, *tail = NULL;
@@ -145,7 +183,11 @@ static int HTS_RESOLVER_CALL mock_getaddrinfo(const char *node,
   *res = NULL;
   if (h == NULL)
     return EAI_NONAME;
+  hts_mutexlock(&mock_lock);
   h->calls++; /* a real backend hit; a cached host skips this */
+  hts_mutexrelease(&mock_lock);
+  if (h->slow_ms != 0)
+    Sleep(h->slow_ms);
   if (h->gai_err != 0)
     return h->gai_err;
   for (int i = 0; i < h->naddr; i++) {
@@ -165,6 +207,18 @@ static int HTS_RESOLVER_CALL mock_getaddrinfo(const char *node,
   return 0;
 }
 
+static int HTS_RESOLVER_CALL mock_getaddrinfo(const char *node,
+                                              const char *service,
+                                              const struct addrinfo *hints,
+                                              struct addrinfo **res) {
+  const int ret = mock_getaddrinfo_(node, service, hints, res);
+
+  hts_mutexlock(&mock_lock);
+  mock_finished++;
+  hts_mutexrelease(&mock_lock);
+  return ret;
+}
+
 static void HTS_RESOLVER_CALL mock_freeaddrinfo(struct addrinfo *res) {
   while (res != NULL) {
     struct addrinfo *const next = res->ai_next;
@@ -370,6 +424,91 @@ int dns_selftests(httrackp *opt) {
   return failures;
 }
 
+/* Probes how long acquiring opt->state.lock takes while a resolve is in
+   flight. hts_has_stopped() takes that same lock and mutates nothing, and the
+   API promises it stays callable from another thread during a mirror. */
+typedef struct lock_probe {
+  httrackp *opt;
+  htsmutex lock;
+  hts_boolean done;
+  TStamp blocked_ms;
+} lock_probe;
+
+static void lock_probe_thread(void *arg) {
+  lock_probe *const p = (lock_probe *) arg;
+  TStamp start;
+
+  Sleep(MOCK_SLOW_MS / 10); /* let the resolve get under way first */
+  start = mtime_local();
+  (void) hts_has_stopped(p->opt);
+  hts_mutexlock(&p->lock);
+  p->blocked_ms = mtime_local() - start;
+  p->done = HTS_TRUE;
+  hts_mutexrelease(&p->lock);
+}
+
+int dns_timeout_selftests(httrackp *opt) {
+  SOCaddr addrs[HTS_MAXADDRNUM];
+  const char *err = NULL;
+  lock_probe probe;
+  TStamp start, elapsed;
+  int count;
+
+  failures = 0;
+  hts_dns_set_resolver_backend(&mock_backend);
+  IPV6_resolver = 0;
+  mock_reset_calls();
+  opt->timeout = 1; /* the bound under test */
+
+  memset(&probe, 0, sizeof(probe));
+  probe.opt = opt;
+  probe.lock = HTSMUTEX_INIT;
+  hts_mutexinit(&probe.lock);
+  CHECK(hts_newthread(lock_probe_thread, &probe) == 0);
+
+  start = mtime_local();
+  count = hts_dns_resolve_all(opt, "slow.test", addrs, HTS_MAXADDRNUM, &err);
+  elapsed = mtime_local() - start;
+
+  /* the resolve returns on opt->timeout, not when the resolver deigns to
+     answer: this is what lets --max-time and --timeout fire (#606). The bound
+     is derived from opt->timeout, never from the mock's sleep, or a resolve
+     that ignored opt->timeout would still pass under the mock. */
+  CHECK(elapsed < (TStamp) opt->timeout * 1000 + 500);
+  CHECK(count == 0); /* a timeout is reported as "does not resolve" */
+
+  /* state.lock is not held across the resolve; a concurrent stop query, which
+     the mirror API promises stays live, is not blocked behind it */
+  for (;;) {
+    hts_boolean done;
+    TStamp blocked;
+
+    hts_mutexlock(&probe.lock);
+    done = probe.done;
+    blocked = probe.blocked_ms;
+    hts_mutexrelease(&probe.lock);
+    if (done) {
+      CHECK(blocked < 500);
+      break;
+    }
+    Sleep(20);
+  }
+  hts_mutexfree(&probe.lock);
+
+  /* a timeout is not an answer, so it must not be negative-cached: the host is
+     resolved again rather than written off for the rest of the crawl */
+  CHECK(mock_read_calls("slow.test") == 1);
+  count = hts_dns_resolve_all(opt, "slow.test", addrs, HTS_MAXADDRNUM, &err);
+  CHECK(count == 0);
+  CHECK(mock_read_calls("slow.test") == 2); /* re-resolved, not cached */
+
+  /* Both resolves were abandoned mid-backend; wait for their workers to leave
+     it before returning. The backend stays installed: an abandoned worker
+     still reads it (to free its addrinfo) after the last call returns. */
+  mock_wait_finished(2);
+  return failures;
+}
+
 #else
 
 int dns_selftests(httrackp *opt) {

src/htsdns_selftest.h

@@ -46,6 +46,12 @@ typedef struct httrackp httrackp;
    Returns the number of failed checks (0 == success). */
 int dns_selftests(httrackp *opt);
 
+/* Drive a deliberately slow (mock) resolver, asserting that a resolve is
+   bounded by opt->timeout, does not hold opt->state.lock while it runs, and
+   does not cache a timeout as an answer (#606). Takes a few seconds.
+   Returns the number of failed checks (0 == success). */
+int dns_timeout_selftests(httrackp *opt);
+
 #endif
 
 #endif

src/htshelp.c

@@ -496,8 +496,8 @@ void help(const char *app, int more) {
   infomsg("");
   infomsg("Flow control:");
   infomsg("  cN number of multiple connections (*c8)");
-  infomsg
-    ("  TN timeout, number of seconds after a non-responding link is shutdown");
+  infomsg("  TN timeout, number of seconds after a non-responding link is"
+          " shutdown; also bounds host name resolution");
   infomsg
     ("  RN number of retries, in case of timeout or non-fatal errors (*R1)");
   infomsg