Просмотр исходного кода

Re-enable getaddrinfo_a with worker-completion wait (#2431) (#2439)

* Restore getaddrinfo_a path with proper worker-completion wait (#2431)

5ebbfee dropped the Linux/glibc getaddrinfo_a branch entirely to avoid
the stack-use-after-free reported in #2431. That sidestepped the bug
but lost the asynchronous-resolution capability getaddrinfo_a is meant
to provide.

Bring the getaddrinfo_a branch back with the actual fix on the
cancellation path: after gai_cancel() — which is non-blocking and may
return EAI_NOTCANCELED while the resolver worker is still mid-operation
— call gai_suspend() with no timeout in a loop until gai_error() stops
returning EAI_INPROGRESS. Only then is it safe to destroy the
stack-local gaicb. freeaddrinfo() is also called on any partially
populated ar_result so that error paths do not leak.

This is the approach suggested in the issue body, with gai_suspend
substituted for the busy-poll over gai_error.

The issue-2431 reproducer test (run under ASAN with sinkhole DNS) is
unchanged and continues to drive the cancel path; it now exercises the
restored getaddrinfo_a code rather than the std::thread fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Simplify getaddrinfo_a branch (idiomatic init, scope_exit, fewer comments)

- Value-initialize gaicb / sigevent / timespec with {} instead of memset
- Replace the two manual freeaddrinfo calls with a scope_exit guard, with
  request.ar_result reset to nullptr on the success path to release
  ownership to the caller (matches the addrinfo cleanup pattern used in
  detail::create_socket and friends)
- Inline the single-call wait_for_request_done lambda
- Drop the (const struct gaicb *const *) cast — the array decays without
  it under C++11
- Tighten the leading comment to the one load-bearing fact (#2431) and
  the trade-off about pathological DNS waits; remove a stale claim that
  the inner loop handles EAI_INTR (the loop checks gai_error, not the
  gai_suspend return value)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yhirose 5 месяцев назад
Родитель
Сommit
806fcb8268
1 измененных файлов с 44 добавлено и 9 удалено
  1. 44 9
      httplib.h

+ 44 - 9
httplib.h

@@ -5906,17 +5906,52 @@ inline int getaddrinfo_with_timeout(const char *node, const char *service,
 
   *res = result_addrinfo;
   return 0;
+#elif defined(_GNU_SOURCE) && defined(__GLIBC__) &&                            \
+    (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 2))
+  // #2431: gai_cancel() is non-blocking and may return EAI_NOTCANCELED while
+  // the resolver worker still references the stack-local gaicb. The cancel
+  // path therefore waits (gai_suspend with no timeout) for the worker to
+  // actually finish before letting the stack frame go. The trade-off is that
+  // a wedged DNS server can hold this thread for the system resolver timeout
+  // (~30s by default) past the caller's connection timeout.
+  struct gaicb request {};
+  struct gaicb *requests[1] = {&request};
+  struct sigevent sevp {};
+  struct timespec timeout {
+    timeout_sec, 0
+  };
+
+  request.ar_name = node;
+  request.ar_service = service;
+  request.ar_request = hints;
+  sevp.sigev_notify = SIGEV_NONE;
+
+  int rc = getaddrinfo_a(GAI_NOWAIT, requests, 1, &sevp);
+  if (rc != 0) { return rc; }
+
+  auto cleanup = scope_exit([&] {
+    if (request.ar_result) { freeaddrinfo(request.ar_result); }
+  });
+
+  int wait_result = gai_suspend(requests, 1, &timeout);
+
+  if (wait_result == 0 || wait_result == EAI_ALLDONE) {
+    int gai_result = gai_error(&request);
+    if (gai_result == 0) {
+      *res = request.ar_result;
+      request.ar_result = nullptr;
+      return 0;
+    }
+    return gai_result;
+  }
+
+  gai_cancel(&request);
+  while (gai_error(&request) == EAI_INPROGRESS) {
+    gai_suspend(requests, 1, nullptr);
+  }
+  return wait_result;
 #else
   // Fallback implementation using thread-based timeout for other Unix systems.
-  //
-  // The previous Linux/glibc path used getaddrinfo_a(GAI_NOWAIT) with a
-  // stack-local gaicb. On timeout it called gai_cancel(), which is non-
-  // blocking and may return EAI_NOTCANCELED -- the resolver worker would
-  // then write back to the destroyed stack frame after this function had
-  // already returned (#2431). The std::thread + shared_ptr fallback below
-  // is correct for the same reason it works on other platforms: the
-  // worker captures shared ownership of the state, so it stays alive as
-  // long as anyone (caller or worker) still holds a reference.
 
   struct GetAddrInfoState {
     ~GetAddrInfoState() {