]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
asyn-thread: fix curl_global_cleanup crash in Windows
authorPavel P <pavlov.pavel@gmail.com>
Thu, 2 May 2024 04:15:44 +0000 (06:15 +0200)
committerJay Satiro <raysatiro@yahoo.com>
Tue, 7 May 2024 18:54:11 +0000 (14:54 -0400)
- Make sure that asynchronous resolves handled by Winsock are stopped
  before WSACleanup is called.

This is implemented by ensuring that when Curl_resolver_kill is called
(eg via multi_done) it will cancel the Winsock asynchronous resolve and
wait for the cancellation to complete. Winsock runs the asynchronous
completion routine immediately when a resolve is canceled.

Prior to this change it was possible that during curl_global_cleanup
"a DNS resolver thread created by GetAddrInfoExW did not terminate yet,
however curl is already shutting down, deinitializing Winsock with
WSACleanup() leading to an access violation."

Background:

If libcurl is built with the asynchronous threaded resolver option for
Windows then it resolves in one of two ways. For Windows 8.1 and later,
libcurl resolves by using the Winsock asynchronous resolver which does
its own thread management. For older versions of Windows, libcurl
resolves by creating a separate thread that calls getaddrinfo. This
change only affects the former and it's already handled for the latter.

Reported-by: Ch40zz@users.noreply.github.com
Fixes https://github.com/curl/curl/issues/13509
Closes https://github.com/curl/curl/pull/13518

lib/asyn-thread.c
lib/curl_threads.c

index f537c0b9f305c76e7df8d8e1ee847d3a4623a960..1760d6cb3f754cb772fca212e147f20322f9a589 100644 (file)
@@ -554,11 +554,15 @@ static void destroy_async_data(struct Curl_async *async)
 
     if(!done) {
 #ifdef _WIN32
-      if(td->complete_ev)
+      if(td->complete_ev) {
         CloseHandle(td->complete_ev);
-      else
+        td->complete_ev = NULL;
+      }
 #endif
-      Curl_thread_destroy(td->thread_hnd);
+      if(td->thread_hnd != curl_thread_t_null) {
+        Curl_thread_destroy(td->thread_hnd);
+        td->thread_hnd = curl_thread_t_null;
+      }
     }
     else {
 #ifdef _WIN32
@@ -566,6 +570,7 @@ static void destroy_async_data(struct Curl_async *async)
         Curl_GetAddrInfoExCancel(&td->tsd.w8.cancel_ev);
         WaitForSingleObject(td->complete_ev, INFINITE);
         CloseHandle(td->complete_ev);
+        td->complete_ev = NULL;
       }
 #endif
       if(td->thread_hnd != curl_thread_t_null)
@@ -713,6 +718,7 @@ static CURLcode thread_wait_resolv(struct Curl_easy *data,
   if(td->complete_ev) {
     WaitForSingleObject(td->complete_ev, INFINITE);
     CloseHandle(td->complete_ev);
+    td->complete_ev = NULL;
     if(entry)
       result = getaddrinfo_complete(data);
   }
@@ -754,6 +760,13 @@ void Curl_resolver_kill(struct Curl_easy *data)
   /* If we're still resolving, we must wait for the threads to fully clean up,
      unfortunately.  Otherwise, we can simply cancel to clean up any resolver
      data. */
+#ifdef _WIN32
+  if(td && td->complete_ev) {
+    Curl_GetAddrInfoExCancel(&td->tsd.w8.cancel_ev);
+    (void)thread_wait_resolv(data, NULL, FALSE);
+  }
+  else
+#endif
   if(td && td->thread_hnd != curl_thread_t_null
      && (data->set.quick_exit != 1L))
     (void)thread_wait_resolv(data, NULL, FALSE);
index 222d9364f03a5e4850828141f0956129e49d3a25..93fa2dafb68a898269a82e3752577c276c1f80e8 100644 (file)
@@ -131,7 +131,8 @@ curl_thread_t Curl_thread_create(unsigned int (CURL_STDCALL *func) (void *),
 
 void Curl_thread_destroy(curl_thread_t hnd)
 {
-  CloseHandle(hnd);
+  if(hnd != curl_thread_t_null)
+    CloseHandle(hnd);
 }
 
 int Curl_thread_join(curl_thread_t *hnd)