From 82009c4220774417c821496affa4aa834c028b68 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Mon, 9 Mar 2026 15:40:34 +0100 Subject: [PATCH] share: concurrency handling, easy updates Replace the `volatile int dirty` with a reference counter protected by a mutex when available. Solve the problem of when to call application's lock function by adding a volatile flag that indicates a share has been added to easy handles in its lifetime. That flag ever goes from FALSE to TRUE, so volatile might work (in the absence of a mutex). (The problem is that the lock/unlock functions need 2-3 `curl_share_setopt()` invocations to become usable and there is no way of telling if the third will ever happen. Calling the lock function before the 3rd setopt may crash the application.) When removing a share from an easy handle (or replacing it with another share), detach the easy connection on a share with a connection pool. When cleaning up a share, allow this even if it is still used in easy handles. It will be destroyed when the reference count drops to 0. Closes #20870 --- CMakeLists.txt | 25 +- configure.ac | 47 ++-- docs/libcurl/curl_share_cleanup.md | 8 +- lib/amigaos.c | 2 +- lib/asyn-ares.c | 4 +- lib/asyn-base.c | 8 +- lib/asyn-thrdd.c | 6 +- lib/asyn.h | 16 +- lib/config-win32.h | 9 +- lib/curl_config-cmake.h.in | 11 +- lib/curl_setup.h | 20 +- lib/curl_share.c | 256 +++++++++++++++++--- lib/curl_share.h | 17 +- lib/curl_threads.c | 16 +- lib/curl_threads.h | 15 +- lib/easy_lock.h | 2 +- lib/hostip.c | 4 +- lib/hostip4.c | 5 +- lib/memdebug.c | 10 +- lib/setopt.c | 70 +----- lib/url.c | 7 +- lib/version.c | 2 +- projects/vms/generate_config_vms_h_curl.com | 4 +- src/curlinfo.c | 4 +- tests/data/test506 | 12 +- tests/libtest/lib3207.c | 8 +- 26 files changed, 378 insertions(+), 210 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1b1f8231d6..339159ed24 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -630,15 +630,20 @@ if(DOS OR AMIGA) set(HAVE_TIME_T_UNSIGNED 1) endif() +if(NOT WIN32) + find_package(Threads) + set(HAVE_THREADS_POSIX ${CMAKE_USE_PTHREADS_INIT}) + set(HAVE_PTHREAD_H ${CMAKE_USE_PTHREADS_INIT}) + list(APPEND CURL_NETWORK_AND_TIME_LIBS ${CMAKE_THREAD_LIBS_INIT}) +endif() + if(ENABLE_THREADED_RESOLVER) - if(WIN32) - set(USE_THREADS_WIN32 ON) - else() - find_package(Threads REQUIRED) - set(USE_THREADS_POSIX ${CMAKE_USE_PTHREADS_INIT}) - set(HAVE_PTHREAD_H ${CMAKE_USE_PTHREADS_INIT}) - list(APPEND CURL_NETWORK_AND_TIME_LIBS ${CMAKE_THREAD_LIBS_INIT}) + if(NOT WIN32 AND NOT HAVE_THREADS_POSIX) + message(FATAL_ERROR "Threaded resolver requires POSIX Threads.") endif() + set(USE_RESOLV_THREADED ON) +elseif(USE_ARES) + set(USE_RESOLV_ARES ON) endif() # Check for all needed libraries @@ -1998,8 +2003,8 @@ curl_add_if("libz" HAVE_LIBZ) curl_add_if("brotli" HAVE_BROTLI) curl_add_if("gsasl" USE_GSASL) curl_add_if("zstd" HAVE_ZSTD) -curl_add_if("AsynchDNS" USE_ARES OR USE_THREADS_POSIX OR USE_THREADS_WIN32) -curl_add_if("asyn-rr" USE_ARES AND ENABLE_THREADED_RESOLVER AND USE_HTTPSRR) +curl_add_if("AsynchDNS" USE_RESOLV_ARES OR USE_RESOLV_THREADED) +curl_add_if("asyn-rr" USE_ARES AND USE_RESOLV_THREADED AND USE_HTTPSRR) curl_add_if("IDN" (HAVE_LIBIDN2 AND HAVE_IDN2_H) OR USE_WIN32_IDN OR USE_APPLE_IDN) @@ -2022,7 +2027,7 @@ curl_add_if("HTTPS-proxy" NOT CURL_DISABLE_PROXY AND _ssl_enabled AND (USE_OPE OR USE_SCHANNEL OR USE_RUSTLS OR USE_MBEDTLS OR (USE_WOLFSSL AND HAVE_WOLFSSL_BIO_NEW))) curl_add_if("Unicode" ENABLE_UNICODE) -curl_add_if("threadsafe" HAVE_ATOMIC OR (USE_THREADS_POSIX AND HAVE_PTHREAD_H) OR WIN32) +curl_add_if("threadsafe" HAVE_ATOMIC OR (HAVE_THREADS_POSIX AND HAVE_PTHREAD_H) OR WIN32) curl_add_if("Debug" ENABLE_DEBUG) curl_add_if("ECH" _ssl_enabled AND HAVE_ECH) curl_add_if("HTTPSRR" _ssl_enabled AND USE_HTTPSRR) diff --git a/configure.ac b/configure.ac index 46df82ed65..2ef223595d 100644 --- a/configure.ac +++ b/configure.ac @@ -4246,15 +4246,8 @@ if test "$ipv6" = "yes" && test "$curl_cv_apple" = "yes"; then CURL_DARWIN_SYSTEMCONFIGURATION fi -dnl Windows threaded resolver check -if test "$want_threaded_resolver" = "yes" && test "$curl_cv_native_windows" = "yes"; then - USE_THREADS_WIN32=1 - AC_DEFINE(USE_THREADS_WIN32, 1, [if you want Win32 threaded DNS lookup]) - curl_res_msg="Win32 threaded" -fi - dnl detect pthreads -if test "$want_threaded_resolver" = "yes" && test "$USE_THREADS_WIN32" != "1"; then +if test "$curl_cv_native_windows" != "yes"; then AC_CHECK_HEADER(pthread.h, [ AC_DEFINE(HAVE_PTHREAD_H, 1, [if you have ]) save_CFLAGS="$CFLAGS" @@ -4267,7 +4260,7 @@ if test "$want_threaded_resolver" = "yes" && test "$USE_THREADS_WIN32" != "1"; t LIBS= dnl Check for libc variants without a separate pthread lib like bionic - AC_CHECK_FUNC(pthread_create, [USE_THREADS_POSIX=1] ) + AC_CHECK_FUNC(pthread_create, [HAVE_THREADS_POSIX=1] ) LIBS="$save_LIBS" case $host in @@ -4279,7 +4272,7 @@ if test "$want_threaded_resolver" = "yes" && test "$USE_THREADS_WIN32" != "1"; t esac dnl if it was not found without lib, search for it in pthread lib - if test "$USE_THREADS_POSIX" != "1"; then + if test "$HAVE_THREADS_POSIX" != "1"; then # assign PTHREAD for pkg-config use PTHREAD=" -pthread" @@ -4302,20 +4295,32 @@ if test "$want_threaded_resolver" = "yes" && test "$USE_THREADS_WIN32" != "1"; t ;; esac AC_CHECK_LIB(pthread, pthread_create, - [USE_THREADS_POSIX=1], + [HAVE_THREADS_POSIX=1], [ CFLAGS="$save_CFLAGS"]) fi - - if test "$USE_THREADS_POSIX" = "1"; then - AC_DEFINE(USE_THREADS_POSIX, 1, [if you want POSIX threaded DNS lookup]) - curl_res_msg="POSIX threaded" - fi ]) + if test "$HAVE_THREADS_POSIX" = "1"; then + AC_DEFINE(HAVE_THREADS_POSIX, 1, [if POSIX pthreads are supported]) + fi fi -dnl Did we find a threading option? -if test "$want_threaded_resolver" != "no" && test "$USE_THREADS_POSIX" != "1" && test "$USE_THREADS_WIN32" != "1"; then - AC_MSG_ERROR([Threaded resolver enabled but no thread library found]) +dnl threaded resolver check +if test "$want_threaded_resolver" = "yes"; then + if test "$curl_cv_native_windows" = "yes"; then + USE_RESOLV_THREADED=1 + AC_DEFINE(USE_RESOLV_THREADED, 1, [if you want threaded DNS lookup]) + curl_res_msg="Win32 threaded" + elif test "$HAVE_THREADS_POSIX" = "1"; then + USE_RESOLV_THREADED=1 + AC_DEFINE(USE_RESOLV_THREADED, 1, [if you want threaded DNS lookup]) + curl_res_msg="POSIX threaded" + else + AC_MSG_ERROR([Threaded resolver enabled but no thread library found]) + fi +elif test "$USE_ARES" = "1"; then + USE_RESOLV_ARES=1 + AC_DEFINE(USE_RESOLV_ARES, 1, [if you want c-ares for DNS lookup]) + curl_res_msg="c-ares" fi AC_CHECK_HEADER(dirent.h, @@ -5157,7 +5162,7 @@ fi if test "$HAVE_ZSTD" = "1"; then SUPPORT_FEATURES="$SUPPORT_FEATURES zstd" fi -if test "$USE_ARES" = "1" || test "$USE_THREADS_POSIX" = "1" || test "$USE_THREADS_WIN32" = "1"; then +if test "$USE_RESOLV_ARES" = "1" || test "$USE_RESOLV_THREADED" = "1"; then SUPPORT_FEATURES="$SUPPORT_FEATURES AsynchDNS" fi if test "$USE_ARES" = "1" && test "$want_threaded_resolver" = "yes" && test "$want_httpsrr" != "no"; then @@ -5291,7 +5296,7 @@ fi if test "$tst_atomic" = "yes"; then SUPPORT_FEATURES="$SUPPORT_FEATURES threadsafe" -elif test "$USE_THREADS_POSIX" = "1" && test "$ac_cv_header_pthread_h" = "yes"; then +elif test "$HAVE_THREADS_POSIX" = "1" && test "$ac_cv_header_pthread_h" = "yes"; then SUPPORT_FEATURES="$SUPPORT_FEATURES threadsafe" elif test "$curl_cv_native_windows" = "yes"; then SUPPORT_FEATURES="$SUPPORT_FEATURES threadsafe" diff --git a/docs/libcurl/curl_share_cleanup.md b/docs/libcurl/curl_share_cleanup.md index afb06d008c..e8390ff3d6 100644 --- a/docs/libcurl/curl_share_cleanup.md +++ b/docs/libcurl/curl_share_cleanup.md @@ -27,7 +27,8 @@ CURLSHcode curl_share_cleanup(CURLSH *share_handle); # DESCRIPTION This function deletes a shared object. The share handle cannot be used anymore -when this function has been called. +when this function has been called. The share fails the call if it is +still being used in any easy handle. Passing in a NULL pointer in *share_handle* makes this function return immediately with no action. @@ -35,6 +36,11 @@ immediately with no action. Any use of the **share_handle** after this function has been called and have returned, is illegal. +For applications that use a share in several threads, it is critical that +the destruction of the share is only done when all other threads have stopped +using it. While libcurl tracks how many easy handles are using a share, +it can not observe how many pointers to the share the application has. + # %PROTOCOLS% # EXAMPLE diff --git a/lib/amigaos.c b/lib/amigaos.c index 31ada5352f..e4f3bfb77c 100644 --- a/lib/amigaos.c +++ b/lib/amigaos.c @@ -141,7 +141,7 @@ struct Curl_addrinfo *Curl_ipv4_resolve_r(const char *hostname, uint16_t port) } } else { -#ifdef CURLRES_THREADED +#ifdef USE_RESOLV_THREADED /* gethostbyname() is not thread-safe, so we need to reopen bsdsocket * on the thread's context */ diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c index b54381f5d6..2599168158 100644 --- a/lib/asyn-ares.c +++ b/lib/asyn-ares.c @@ -23,7 +23,7 @@ ***************************************************************************/ #include "curl_setup.h" -#ifdef CURLRES_ARES +#ifdef USE_RESOLV_ARES /*********************************************************************** * Only for ares-enabled builds and only for functions that fulfill @@ -966,4 +966,4 @@ CURLcode Curl_async_ares_set_dns_local_ip6(struct Curl_easy *data) #endif } -#endif /* CURLRES_ARES */ +#endif /* USE_RESOLV_ARES */ diff --git a/lib/asyn-base.c b/lib/asyn-base.c index 2524b4e9f8..ee85111d6a 100644 --- a/lib/asyn-base.c +++ b/lib/asyn-base.c @@ -183,10 +183,10 @@ void Curl_async_shutdown(struct Curl_easy *data) { if(data->state.async) { CURL_TRC_DNS(data, "shutdown async"); -#ifdef CURLRES_ARES +#ifdef USE_RESOLV_ARES Curl_async_ares_shutdown(data, data->state.async); #endif -#ifdef CURLRES_THREADED +#ifdef USE_RESOLV_THREADED Curl_async_thrdd_shutdown(data, data->state.async); #endif #ifndef CURL_DISABLE_DOH @@ -199,10 +199,10 @@ void Curl_async_destroy(struct Curl_easy *data) { if(data->state.async) { CURL_TRC_DNS(data, "destroy async"); -#ifdef CURLRES_ARES +#ifdef USE_RESOLV_ARES Curl_async_ares_destroy(data, data->state.async); #endif -#ifdef CURLRES_THREADED +#ifdef USE_RESOLV_THREADED Curl_async_thrdd_destroy(data, data->state.async); #endif #ifndef CURL_DISABLE_DOH diff --git a/lib/asyn-thrdd.c b/lib/asyn-thrdd.c index 5984e7bc3a..22418f5291 100644 --- a/lib/asyn-thrdd.c +++ b/lib/asyn-thrdd.c @@ -26,7 +26,7 @@ /*********************************************************************** * Only for threaded name resolves builds **********************************************************************/ -#ifdef CURLRES_THREADED +#ifdef USE_RESOLV_THREADED #include "socketpair.h" @@ -44,7 +44,7 @@ #include #endif -#if defined(USE_THREADS_POSIX) && defined(HAVE_PTHREAD_H) +#if defined(HAVE_THREADS_POSIX) && defined(HAVE_PTHREAD_H) #include #endif @@ -739,4 +739,4 @@ CURLcode Curl_async_getaddrinfo(struct Curl_easy *data, #endif /* !HAVE_GETADDRINFO */ -#endif /* CURLRES_THREADED */ +#endif /* USE_RESOLV_THREADED */ diff --git a/lib/asyn.h b/lib/asyn.h index 7bc71eab56..54b49f96f3 100644 --- a/lib/asyn.h +++ b/lib/asyn.h @@ -41,8 +41,8 @@ struct hostent; struct connectdata; struct easy_pollset; -#if defined(CURLRES_ARES) && defined(CURLRES_THREADED) -#error cannot have both CURLRES_ARES and CURLRES_THREADED defined +#if defined(USE_RESOLV_ARES) && defined(USE_RESOLV_THREADED) +#error cannot have both USE_RESOLV_ARES and USE_RESOLV_THREADED defined #endif /* @@ -134,7 +134,7 @@ CURLcode Curl_ares_pollset(struct Curl_easy *data, int Curl_ares_perform(ares_channel channel, timediff_t timeout_ms); #endif -#ifdef CURLRES_ARES +#ifdef USE_RESOLV_ARES /* async resolving implementation using c-ares alone */ struct async_ares_ctx { ares_channel channel; @@ -168,9 +168,9 @@ CURLcode Curl_async_ares_set_dns_local_ip4(struct Curl_easy *data); /* Set the local ipv6 address to use by ares, from `data` settings. */ CURLcode Curl_async_ares_set_dns_local_ip6(struct Curl_easy *data); -#endif /* CURLRES_ARES */ +#endif /* USE_RESOLV_ARES */ -#ifdef CURLRES_THREADED +#ifdef USE_RESOLV_THREADED /* async resolving implementation using POSIX threads */ #include "curl_threads.h" @@ -219,7 +219,7 @@ void Curl_async_thrdd_shutdown(struct Curl_easy *data, void Curl_async_thrdd_destroy(struct Curl_easy *data, struct Curl_resolv_async *async); -#endif /* CURLRES_THREADED */ +#endif /* USE_RESOLV_THREADED */ #ifndef CURL_DISABLE_DOH struct doh_probes; @@ -242,9 +242,9 @@ struct doh_probes; #ifdef USE_CURL_ASYNC struct Curl_resolv_async { -#ifdef CURLRES_ARES +#ifdef USE_RESOLV_ARES struct async_ares_ctx ares; -#elif defined(CURLRES_THREADED) +#elif defined(USE_RESOLV_THREADED) struct async_thrdd_ctx thrdd; #endif #ifndef CURL_DISABLE_DOH diff --git a/lib/config-win32.h b/lib/config-win32.h index ddde06a3ef..4126726716 100644 --- a/lib/config-win32.h +++ b/lib/config-win32.h @@ -262,15 +262,16 @@ /* ---------------------------------------------------------------- */ /* - * Undefine both USE_ARES and USE_THREADS_WIN32 for synchronous DNS. + * Undefine both USE_ARES and USE_RESOLV_THREADED for synchronous DNS. */ /* Default define to enable threaded asynchronous DNS lookups. */ -#if !defined(USE_SYNC_DNS) && !defined(USE_ARES) && !defined(USE_THREADS_WIN32) -# define USE_THREADS_WIN32 1 +#if !defined(USE_SYNC_DNS) && !defined(USE_ARES) && \ + !defined(USE_RESOLV_THREADED) +# define USE_RESOLV_THREADED 1 #endif -#if defined(USE_ARES) && defined(USE_THREADS_WIN32) +#if defined(USE_ARES) && defined(USE_RESOLV_THREADED) # error "Only one DNS lookup specialty may be defined at most" #endif diff --git a/lib/curl_config-cmake.h.in b/lib/curl_config-cmake.h.in index fec13419b7..ac9080fe54 100644 --- a/lib/curl_config-cmake.h.in +++ b/lib/curl_config-cmake.h.in @@ -643,14 +643,17 @@ ${SIZEOF_TIME_T_CODE} /* Define to 1 if you have the ANSI C header files. */ #cmakedefine STDC_HEADERS 1 +/* Define if you have POSIX pthreads */ +#cmakedefine HAVE_THREADS_POSIX 1 + /* Define if you want to enable c-ares support */ #cmakedefine USE_ARES 1 -/* Define if you want to enable POSIX threaded DNS lookup */ -#cmakedefine USE_THREADS_POSIX 1 +/* Define if you want to enable c-ares DNS lookup */ +#cmakedefine USE_RESOLV_ARES 1 -/* Define if you want to enable Win32 threaded DNS lookup */ -#cmakedefine USE_THREADS_WIN32 1 +/* Define if you want to enable threaded DNS lookup */ +#cmakedefine USE_RESOLV_THREADED 1 /* if GnuTLS is enabled */ #cmakedefine USE_GNUTLS 1 diff --git a/lib/curl_setup.h b/lib/curl_setup.h index 0765c80fc0..83ee65c68c 100644 --- a/lib/curl_setup.h +++ b/lib/curl_setup.h @@ -220,7 +220,7 @@ /* ================================================================ */ /* Give calloc a chance to be dragging in early, so we do not redefine */ -#if defined(USE_THREADS_POSIX) && defined(HAVE_PTHREAD_H) +#if defined(HAVE_THREADS_POSIX) && defined(HAVE_PTHREAD_H) # include #endif @@ -442,7 +442,7 @@ # if !(defined(__NEWLIB__) || \ (defined(__CLIB2__) && defined(__THREAD_SAFE))) /* disable threaded resolver with clib2 - requires newlib or clib-ts */ -# undef USE_THREADS_POSIX +# undef USE_RESOLV_THREADED # endif # endif # include @@ -685,6 +685,16 @@ #endif /* _WIN32 */ +/* We want to use mutex when available. */ +#if defined(HAVE_THREADS_POSIX) || defined(_WIN32) +#define USE_MUTEX +#endif + +/* threaded resolver is the only feature requiring threads. */ +#ifdef USE_RESOLV_THREADED +#define USE_THREADS +#endif + /* ---------------------------------------------------------------- */ /* resolver specialty compile-time defines */ /* CURLRES_* defines to use in the host*.c sources */ @@ -702,12 +712,10 @@ # define CURLRES_IPV4 #endif -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_RESOLV_THREADED # define CURLRES_ASYNCH -# define CURLRES_THREADED -#elif defined(USE_ARES) +#elif defined(USE_RESOLV_ARES) # define CURLRES_ASYNCH -# define CURLRES_ARES /* now undef the stock libc functions to avoid them being used */ # undef HAVE_GETADDRINFO # undef HAVE_FREEADDRINFO diff --git a/lib/curl_share.c b/lib/curl_share.c index 0f193004a2..ab815abc8f 100644 --- a/lib/curl_share.c +++ b/lib/curl_share.c @@ -24,22 +24,61 @@ #include "curl_setup.h" #include "urldata.h" +#include "multiif.h" +#include "curl_threads.h" #include "curl_share.h" #include "vtls/vtls.h" #include "vtls/vtls_scache.h" #include "hsts.h" #include "url.h" +static void share_destroy(struct Curl_share *share) +{ + if(share->specifier & (1 << CURL_LOCK_DATA_CONNECT)) { + Curl_cpool_destroy(&share->cpool); + } + + Curl_dnscache_destroy(&share->dnscache); + +#if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES) + Curl_cookie_cleanup(share->cookies); +#endif + +#ifndef CURL_DISABLE_HSTS + Curl_hsts_cleanup(&share->hsts); +#endif + +#ifdef USE_SSL + if(share->ssl_scache) { + Curl_ssl_scache_destroy(share->ssl_scache); + share->ssl_scache = NULL; + } +#endif + + Curl_psl_destroy(&share->psl); + Curl_close(&share->admin); + +#ifdef USE_MUTEX + Curl_mutex_destroy(&share->lock); +#endif + share->magic = 0; + curlx_free(share); +} + CURLSH *curl_share_init(void) { struct Curl_share *share = curlx_calloc(1, sizeof(struct Curl_share)); if(share) { share->magic = CURL_GOOD_SHARE; share->specifier |= (1 << CURL_LOCK_DATA_SHARE); +#ifdef USE_MUTEX + Curl_mutex_init(&share->lock); +#endif + share->ref_count = 1; Curl_dnscache_init(&share->dnscache, 23); share->admin = curl_easy_init(); if(!share->admin) { - curlx_free(share); + share_destroy(share); return NULL; } /* admin handles have mid 0 */ @@ -50,10 +89,104 @@ CURLSH *curl_share_init(void) share->admin->set.verbose = TRUE; #endif } - return share; } +static uint32_t share_ref_inc(struct Curl_share *share) +{ + uint32_t n; +#ifdef USE_MUTEX + Curl_mutex_acquire(&share->lock); + n = ++(share->ref_count); + share->has_been_shared = TRUE; + Curl_mutex_release(&share->lock); +#else + n = ++(share->ref_count); + share->has_been_shared = TRUE; +#endif + return n; +} + +static uint32_t share_ref_dec(struct Curl_share *share) +{ + uint32_t n; +#ifdef USE_MUTEX + Curl_mutex_acquire(&share->lock); + DEBUGASSERT(share->ref_count); + n = --(share->ref_count); + Curl_mutex_release(&share->lock); +#else + n = --(share->ref_count); +#endif + return n; +} + +static bool share_has_been_shared(struct Curl_share *share) +{ + bool was_shared; +#ifdef USE_MUTEX + Curl_mutex_acquire(&share->lock); + was_shared = share->has_been_shared; + Curl_mutex_release(&share->lock); +#else + was_shared = share->has_been_shared; +#endif + return was_shared; +} + +static bool share_lock_acquire(struct Curl_share *share, + struct Curl_easy *data) +{ + if(share->lockfunc && share->unlockfunc && + (data || share_has_been_shared(share))) { + share->lockfunc(data, CURL_LOCK_DATA_SHARE, CURL_LOCK_ACCESS_SINGLE, + share->clientdata); + return TRUE; + } + return FALSE; +} + +static void share_lock_release(struct Curl_share *share, + struct Curl_easy *data, + bool locked) +{ + if(locked) { + DEBUGASSERT(share->unlockfunc); + if(share->unlockfunc) + share->unlockfunc(data, CURL_LOCK_DATA_SHARE, share->clientdata); + } +} + +static bool share_in_use(struct Curl_share *share) +{ + bool in_use; +#ifdef USE_MUTEX + Curl_mutex_acquire(&share->lock); + in_use = (share->ref_count > 1); + Curl_mutex_release(&share->lock); +#else + bool locked = share_lock_acquire(share, NULL); + in_use = (share->ref_count > 1); + share_lock_release(share, NULL, locked); +#endif + return in_use; +} + +static void share_unlink(struct Curl_share **pshare, + struct Curl_easy *data, + bool locked) +{ + struct Curl_share *share = *pshare; + uint32_t n; + + *pshare = NULL; + n = share_ref_dec(share); + if(locked) + share_lock_release(share, data, locked); + if(!n) /* last reference gone */ + share_destroy(share); +} + #undef curl_share_setopt CURLSHcode curl_share_setopt(CURLSH *sh, CURLSHoption option, ...) { @@ -68,10 +201,11 @@ CURLSHcode curl_share_setopt(CURLSH *sh, CURLSHoption option, ...) if(!GOOD_SHARE_HANDLE(share)) return CURLSHE_INVALID; - if(share->dirty) + if(share_in_use(share)) { /* do not allow setting options while one or more handles are already using this share */ return CURLSHE_IN_USE; + } va_start(param, option); @@ -221,48 +355,15 @@ CURLSHcode curl_share_setopt(CURLSH *sh, CURLSHoption option, ...) CURLSHcode curl_share_cleanup(CURLSH *sh) { struct Curl_share *share = sh; + bool locked; if(!GOOD_SHARE_HANDLE(share)) return CURLSHE_INVALID; - if(share->lockfunc) - share->lockfunc(NULL, CURL_LOCK_DATA_SHARE, CURL_LOCK_ACCESS_SINGLE, - share->clientdata); - - if(share->dirty) { - if(share->unlockfunc) - share->unlockfunc(NULL, CURL_LOCK_DATA_SHARE, share->clientdata); + if(share_in_use(share)) return CURLSHE_IN_USE; - } - - if(share->specifier & (1 << CURL_LOCK_DATA_CONNECT)) { - Curl_cpool_destroy(&share->cpool); - } - - Curl_dnscache_destroy(&share->dnscache); - -#if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES) - Curl_cookie_cleanup(share->cookies); -#endif - -#ifndef CURL_DISABLE_HSTS - Curl_hsts_cleanup(&share->hsts); -#endif - -#ifdef USE_SSL - if(share->ssl_scache) { - Curl_ssl_scache_destroy(share->ssl_scache); - share->ssl_scache = NULL; - } -#endif - - Curl_psl_destroy(&share->psl); - Curl_close(&share->admin); - - if(share->unlockfunc) - share->unlockfunc(NULL, CURL_LOCK_DATA_SHARE, share->clientdata); - share->magic = 0; - curlx_free(share); + locked = share_lock_acquire(share, NULL); + share_unlink(&share, NULL, locked); return CURLSHE_OK; } @@ -297,3 +398,78 @@ CURLSHcode Curl_share_unlock(struct Curl_easy *data, curl_lock_data type) return CURLSHE_OK; } + +CURLcode Curl_share_easy_unlink(struct Curl_easy *data) +{ + struct Curl_share *share = data->share; + + if(share) { + bool locked = share_lock_acquire(share, data); + + /* If data has a connection from this share, detach it. */ + if(data->conn && (share->specifier & (1 << CURL_LOCK_DATA_CONNECT))) + Curl_detach_connection(data); + +#if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES) + if(share->cookies == data->cookies) + data->cookies = NULL; +#endif + +#ifndef CURL_DISABLE_HSTS + if(share->hsts == data->hsts) + data->hsts = NULL; +#endif +#ifdef USE_LIBPSL + if(&share->psl == data->psl) + data->psl = data->multi ? &data->multi->psl : NULL; +#endif + if(share->specifier & (1 << CURL_LOCK_DATA_DNS)) { + Curl_dns_entry_unlink(data, &data->state.dns[0]); + Curl_dns_entry_unlink(data, &data->state.dns[1]); + } + + share_unlink(&data->share, data, locked); + } + return CURLE_OK; +} + +CURLcode Curl_share_easy_link(struct Curl_easy *data, + struct Curl_share *share) +{ + if(data->share) { + DEBUGASSERT(0); + return CURLE_FAILED_INIT; + } + + if(share) { + bool locked = share_lock_acquire(share, data); + + share_ref_inc(share); + data->share = share; + +#if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES) + if(share->cookies) { + /* use shared cookie list, first free own one if any */ + Curl_cookie_cleanup(data->cookies); + /* enable cookies since we now use a share that uses cookies! */ + data->cookies = share->cookies; + } +#endif /* CURL_DISABLE_HTTP */ +#ifndef CURL_DISABLE_HSTS + if(share->hsts) { + /* first free the private one if any */ + Curl_hsts_cleanup(&data->hsts); + data->hsts = share->hsts; + } +#endif +#ifdef USE_LIBPSL + if(share->specifier & (1 << CURL_LOCK_DATA_PSL)) + data->psl = &share->psl; +#endif + + /* check for host cache not needed, + * it will be done by curl_easy_perform */ + share_lock_release(share, data, locked); + } + return CURLE_OK; +} diff --git a/lib/curl_share.h b/lib/curl_share.h index 5e3f82aa56..69001be705 100644 --- a/lib/curl_share.h +++ b/lib/curl_share.h @@ -25,6 +25,7 @@ ***************************************************************************/ #include "curl_setup.h" +#include "curl_threads.h" #include "cookie.h" #include "psl.h" #include "urldata.h" @@ -43,12 +44,22 @@ struct Curl_ssl_scache; struct Curl_share { unsigned int magic; /* CURL_GOOD_SHARE */ unsigned int specifier; - volatile unsigned int dirty; + uint32_t ref_count; +#ifdef USE_MUTEX + /* do `ref_count` and `has_been_shared` checks using this mutex. */ + curl_mutex_t lock; + int has_been_shared; +#else + /* this only ever goes from FALSE -> TRUE once. We need to check + * this without being able to use the `lockfunc`. */ + volatile int has_been_shared; +#endif curl_lock_function lockfunc; curl_unlock_function unlockfunc; void *clientdata; struct Curl_easy *admin; + struct cpool cpool; struct Curl_dnscache dnscache; /* DNS cache */ #if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES) @@ -74,4 +85,8 @@ CURLSHcode Curl_share_unlock(struct Curl_easy *data, curl_lock_data type); ((data)->share->specifier & \ (1 << CURL_LOCK_DATA_SSL_SESSION))) +CURLcode Curl_share_easy_unlink(struct Curl_easy *data); +CURLcode Curl_share_easy_link(struct Curl_easy *data, + struct Curl_share *share); + #endif /* HEADER_CURL_SHARE_H */ diff --git a/lib/curl_threads.c b/lib/curl_threads.c index f5fea98046..52ab385a44 100644 --- a/lib/curl_threads.c +++ b/lib/curl_threads.c @@ -22,14 +22,11 @@ * ***************************************************************************/ #include "curl_setup.h" - -#if defined(USE_THREADS_POSIX) && defined(HAVE_PTHREAD_H) -#include -#endif - #include "curl_threads.h" -#ifdef USE_THREADS_POSIX +#ifdef USE_THREADS + +#ifdef HAVE_THREADS_POSIX struct Curl_actual_call { unsigned int (*func)(void *); @@ -97,7 +94,7 @@ int Curl_thread_join(curl_thread_t *hnd) return ret; } -#elif defined(USE_THREADS_WIN32) +#elif defined(_WIN32) curl_thread_t Curl_thread_create(CURL_THREAD_RETURN_T (CURL_STDCALL *func) (void *), void *arg) @@ -131,4 +128,7 @@ int Curl_thread_join(curl_thread_t *hnd) return ret; } -#endif /* USE_THREADS_* */ +#else +#error neither HAVE_THREADS_POSIX nor _WIN32 defined +#endif +#endif /* USE_THREADS */ diff --git a/lib/curl_threads.h b/lib/curl_threads.h index 0e067c059a..ff3af6a793 100644 --- a/lib/curl_threads.h +++ b/lib/curl_threads.h @@ -25,7 +25,11 @@ ***************************************************************************/ #include "curl_setup.h" -#ifdef USE_THREADS_POSIX +#ifdef USE_MUTEX +#ifdef HAVE_THREADS_POSIX +#ifdef HAVE_PTHREAD_H +#include +#endif # define CURL_THREAD_RETURN_T unsigned int # define CURL_STDCALL # define curl_mutex_t pthread_mutex_t @@ -35,7 +39,7 @@ # define Curl_mutex_acquire(m) pthread_mutex_lock(m) # define Curl_mutex_release(m) pthread_mutex_unlock(m) # define Curl_mutex_destroy(m) pthread_mutex_destroy(m) -#elif defined(USE_THREADS_WIN32) +#elif defined(_WIN32) # define CURL_THREAD_RETURN_T DWORD # define CURL_STDCALL WINAPI # define curl_mutex_t CRITICAL_SECTION @@ -45,9 +49,12 @@ # define Curl_mutex_acquire(m) EnterCriticalSection(m) # define Curl_mutex_release(m) LeaveCriticalSection(m) # define Curl_mutex_destroy(m) DeleteCriticalSection(m) +#else +#error neither HAVE_THREADS_POSIX nor _WIN32 defined #endif +#endif /* USE_MUTEX */ -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_THREADS curl_thread_t Curl_thread_create(CURL_THREAD_RETURN_T (CURL_STDCALL *func) (void *), void *arg); @@ -56,6 +63,6 @@ void Curl_thread_destroy(curl_thread_t *hnd); int Curl_thread_join(curl_thread_t *hnd); -#endif /* USE_THREADS_POSIX || USE_THREADS_WIN32 */ +#endif /* USE_THREADS */ #endif /* HEADER_CURL_THREADS_H */ diff --git a/lib/easy_lock.h b/lib/easy_lock.h index cefb01ddc8..aa2fea705c 100644 --- a/lib/easy_lock.h +++ b/lib/easy_lock.h @@ -84,7 +84,7 @@ static CURL_INLINE void curl_simple_lock_unlock(curl_simple_lock *lock) atomic_store_explicit(lock, false, memory_order_release); } -#elif defined(USE_THREADS_POSIX) && defined(HAVE_PTHREAD_H) +#elif defined(HAVE_THREADS_POSIX) && defined(HAVE_PTHREAD_H) #include diff --git a/lib/hostip.c b/lib/hostip.c index 6406a850af..648fcbc965 100644 --- a/lib/hostip.c +++ b/lib/hostip.c @@ -86,10 +86,10 @@ * take that into account. Hosts that are not IPv6-enabled have CURLRES_IPV4 * defined. * - * CURLRES_ARES - is defined if libcurl is built to use c-ares for + * USE_RESOLV_ARES - is defined if libcurl is built to use c-ares for * asynchronous name resolves. This can be Windows or *nix. * - * CURLRES_THREADED - is defined if libcurl is built to run under (native) + * USE_RESOLV_THREADED - is defined if libcurl is built to run under (native) * Windows, and then the name resolve will be done in a new thread, and the * supported API will be the same as for ares-builds. * diff --git a/lib/hostip4.c b/lib/hostip4.c index 114b9aca58..fdf3825397 100644 --- a/lib/hostip4.c +++ b/lib/hostip4.c @@ -85,7 +85,8 @@ struct Curl_addrinfo *Curl_sync_getaddrinfo(struct Curl_easy *data, #endif /* CURLRES_SYNCH */ #endif /* CURLRES_IPV4 */ -#if defined(CURLRES_IPV4) && !defined(CURLRES_ARES) && !defined(CURLRES_AMIGA) +#if defined(CURLRES_IPV4) && !defined(USE_RESOLV_ARES) && \ + !defined(CURLRES_AMIGA) /* * Curl_ipv4_resolve_r() - ipv4 thread-safe resolver function. @@ -277,4 +278,4 @@ struct Curl_addrinfo *Curl_ipv4_resolve_r(const char *hostname, return ai; } -#endif /* CURLRES_IPV4 && !CURLRES_ARES && !CURLRES_AMIGA */ +#endif /* CURLRES_IPV4 && !USE_RESOLV_ARES && !CURLRES_AMIGA */ diff --git a/lib/memdebug.c b/lib/memdebug.c index af4fc9d6c1..408930179d 100644 --- a/lib/memdebug.c +++ b/lib/memdebug.c @@ -65,14 +65,14 @@ static struct backtrace_state *btstate; static char membuf[10000]; static size_t memwidx = 0; /* write index */ -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_MUTEX static bool dbg_mutex_init = 0; static curl_mutex_t dbg_mutex; #endif static bool curl_dbg_lock(void) { -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_MUTEX if(dbg_mutex_init) { Curl_mutex_acquire(&dbg_mutex); return TRUE; @@ -83,7 +83,7 @@ static bool curl_dbg_lock(void) static void curl_dbg_unlock(bool was_locked) { -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_MUTEX if(was_locked) Curl_mutex_release(&dbg_mutex); #else @@ -108,7 +108,7 @@ static void curl_dbg_cleanup(void) fclose(curl_dbg_logfile); } curl_dbg_logfile = NULL; -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_MUTEX if(dbg_mutex_init) { Curl_mutex_destroy(&dbg_mutex); dbg_mutex_init = FALSE; @@ -157,7 +157,7 @@ void curl_dbg_memdebug(const char *logname) setbuf(curl_dbg_logfile, (char *)NULL); #endif } -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_MUTEX if(!dbg_mutex_init) { dbg_mutex_init = TRUE; Curl_mutex_init(&dbg_mutex); diff --git a/lib/setopt.c b/lib/setopt.c index a48c6a8693..a179a48a9a 100644 --- a/lib/setopt.c +++ b/lib/setopt.c @@ -1490,67 +1490,17 @@ static CURLcode setopt_pointers(struct Curl_easy *data, CURLoption option, case CURLOPT_SHARE: { struct Curl_share *set = va_arg(param, struct Curl_share *); - /* disconnect from old share, if any */ - if(data->share) { - Curl_share_lock(data, CURL_LOCK_DATA_SHARE, CURL_LOCK_ACCESS_SINGLE); + /* disconnect from old share, if any and possible */ + result = Curl_share_easy_unlink(data); + if(result) + return result; -#if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES) - if(data->share->cookies == data->cookies) - data->cookies = NULL; -#endif - -#ifndef CURL_DISABLE_HSTS - if(data->share->hsts == data->hsts) - data->hsts = NULL; -#endif -#ifdef USE_LIBPSL - if(data->psl == &data->share->psl) - data->psl = data->multi ? &data->multi->psl : NULL; -#endif - if(data->share->specifier & (1 << CURL_LOCK_DATA_DNS)) { - Curl_dns_entry_unlink(data, &data->state.dns[0]); - Curl_dns_entry_unlink(data, &data->state.dns[1]); - } - - data->share->dirty--; - - Curl_share_unlock(data, CURL_LOCK_DATA_SHARE); - data->share = NULL; + /* use new share if it set */ + if(GOOD_SHARE_HANDLE(set)) { + result = Curl_share_easy_link(data, set); + if(result) + return result; } - - if(GOOD_SHARE_HANDLE(set)) - /* use new share if it set */ - data->share = set; - if(data->share) { - - Curl_share_lock(data, CURL_LOCK_DATA_SHARE, CURL_LOCK_ACCESS_SINGLE); - - data->share->dirty++; - -#if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_COOKIES) - if(data->share->cookies) { - /* use shared cookie list, first free own one if any */ - Curl_cookie_cleanup(data->cookies); - /* enable cookies since we now use a share that uses cookies! */ - data->cookies = data->share->cookies; - } -#endif /* CURL_DISABLE_HTTP */ -#ifndef CURL_DISABLE_HSTS - if(data->share->hsts) { - /* first free the private one if any */ - Curl_hsts_cleanup(&data->hsts); - data->hsts = data->share->hsts; - } -#endif -#ifdef USE_LIBPSL - if(data->share->specifier & (1 << CURL_LOCK_DATA_PSL)) - data->psl = &data->share->psl; -#endif - - Curl_share_unlock(data, CURL_LOCK_DATA_SHARE); - } - /* check for host cache not needed, - * it will be done by curl_easy_perform */ break; } @@ -2434,7 +2384,7 @@ static CURLcode setopt_cptr(struct Curl_easy *data, CURLoption option, break; #endif #endif -#ifdef CURLRES_ARES +#ifdef USE_RESOLV_ARES case CURLOPT_DNS_SERVERS: result = Curl_setstropt(&s->str[STRING_DNS_SERVERS], ptr); if(result) diff --git a/lib/url.c b/lib/url.c index e156e01b6b..0e068f08ee 100644 --- a/lib/url.c +++ b/lib/url.c @@ -287,11 +287,8 @@ CURLcode Curl_close(struct Curl_easy **datap) data_priority_cleanup(data); /* No longer a dirty share, if it exists */ - if(data->share) { - Curl_share_lock(data, CURL_LOCK_DATA_SHARE, CURL_LOCK_ACCESS_SINGLE); - data->share->dirty--; - Curl_share_unlock(data, CURL_LOCK_DATA_SHARE); - } + if(Curl_share_easy_unlink(data)) + DEBUGASSERT(0); Curl_hash_destroy(&data->meta_hash); #ifndef CURL_DISABLE_PROXY diff --git a/lib/version.c b/lib/version.c index 7e54bde26d..447dda0342 100644 --- a/lib/version.c +++ b/lib/version.c @@ -451,7 +451,7 @@ static const struct feat features_table[] = { #ifndef CURL_DISABLE_ALTSVC FEATURE("alt-svc", NULL, CURL_VERSION_ALTSVC), #endif -#if defined(USE_ARES) && defined(CURLRES_THREADED) && defined(USE_HTTPSRR) +#if defined(USE_ARES) && defined(USE_RESOLV_THREADED) && defined(USE_HTTPSRR) FEATURE("asyn-rr", NULL, 0), #endif #ifdef CURLRES_ASYNCH diff --git a/projects/vms/generate_config_vms_h_curl.com b/projects/vms/generate_config_vms_h_curl.com index 245d072d20..d5fc24059a 100644 --- a/projects/vms/generate_config_vms_h_curl.com +++ b/projects/vms/generate_config_vms_h_curl.com @@ -327,8 +327,8 @@ $write cvh "#endif" $write cvh "#ifdef USE_OPENLDAP" $write cvh "#undef USE_OPENLDAP" $write cvh "#endif" -$write cvh "#ifdef USE_THREADS_POSIX" -$write cvh "#undef USE_THREADS_POSIX" +$write cvh "#ifdef USE_RESOLV_THREADED" +$write cvh "#undef USE_RESOLV_THREADED" $write cvh "#endif" $write cvh "#ifdef USE_TLS_SRP" $write cvh "#undef USE_TLS_SRP" diff --git a/src/curlinfo.c b/src/curlinfo.c index 0a6c7fc135..b34e27fb88 100644 --- a/src/curlinfo.c +++ b/src/curlinfo.c @@ -34,7 +34,7 @@ #include "multihandle.h" /* for ENABLE_WAKEUP */ #include "tool_xattr.h" /* for USE_XATTR */ #include "curl_sha512_256.h" /* for CURL_HAVE_SHA512_256 */ -#include "asyn.h" /* for CURLRES_ARES */ +#include "asyn.h" /* for USE_RESOLV_ARES */ #include "fake_addrinfo.h" /* for USE_FAKE_GETADDRINFO */ #include @@ -235,7 +235,7 @@ static const char *disabled[] = { , "override-dns: " #if defined(CURL_MEMDEBUG) && \ - (defined(CURLRES_ARES) || defined(USE_FAKE_GETADDRINFO)) + (defined(USE_RESOLV_ARES) || defined(USE_FAKE_GETADDRINFO)) "ON" #else "OFF" diff --git a/tests/data/test506 b/tests/data/test506 index 3455a7b4ca..4440a7a582 100644 --- a/tests/data/test506 +++ b/tests/data/test506 @@ -227,17 +227,15 @@ loaded cookies: .host.foo.com TRUE / FALSE %days[400] injected yes ----------------- try SHARE_CLEANUP... -lock: share [Pigs in space]: 98 -unlock: share [Pigs in space]: 99 SHARE_CLEANUP failed, correct CLEANUP -lock: cookie [Pigs in space]: 100 -unlock: cookie [Pigs in space]: 101 +lock: cookie [Pigs in space]: 98 +unlock: cookie [Pigs in space]: 99 +lock: share [Pigs in space]: 100 +unlock: share [Pigs in space]: 101 +SHARE_CLEANUP lock: share [Pigs in space]: 102 unlock: share [Pigs in space]: 103 -SHARE_CLEANUP -lock: share [Pigs in space]: 104 -unlock: share [Pigs in space]: 105 GLOBAL_CLEANUP diff --git a/tests/libtest/lib3207.c b/tests/libtest/lib3207.c index c764add27f..ae2a3a18d1 100644 --- a/tests/libtest/lib3207.c +++ b/tests/libtest/lib3207.c @@ -23,10 +23,6 @@ ***************************************************************************/ #include "first.h" -#ifdef USE_THREADS_POSIX -#include -#endif - #include "curl_threads.h" #define THREAD_SIZE 16 @@ -66,7 +62,7 @@ static size_t write_memory_callback(char *contents, size_t size, return realsize; } -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_THREADS static CURL_THREAD_RETURN_T CURL_STDCALL test_thread(void *ptr) #else static unsigned int test_thread(void *ptr) @@ -111,7 +107,7 @@ test_cleanup: return 0; } -#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32) +#ifdef USE_THREADS static void t3207_test_lock(CURL *curl, curl_lock_data data, curl_lock_access laccess, void *useptr)