diff --git a/CMakeLists.txt b/CMakeLists.txt index be58defb..ae6a92f0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -33,14 +33,15 @@ INCLUDE (GNUInstallDirs) # include this *AFTER* PROJECT(), otherwise paths are w SET (CARES_LIB_VERSIONINFO "10:1:8") -OPTION (CARES_STATIC "Build as a static library" OFF) -OPTION (CARES_SHARED "Build as a shared library" ON) -OPTION (CARES_INSTALL "Create installation targets (chain builders may want to disable this)" ON) -OPTION (CARES_STATIC_PIC "Build the static library as PIC (position independent)" OFF) -OPTION (CARES_BUILD_TESTS "Build and run tests" OFF) +OPTION (CARES_STATIC "Build as a static library" OFF) +OPTION (CARES_SHARED "Build as a shared library" ON) +OPTION (CARES_INSTALL "Create installation targets (chain builders may want to disable this)" ON) +OPTION (CARES_STATIC_PIC "Build the static library as PIC (position independent)" OFF) +OPTION (CARES_BUILD_TESTS "Build and run tests" OFF) OPTION (CARES_BUILD_CONTAINER_TESTS "Build and run container tests (implies CARES_BUILD_TESTS, Linux only)" OFF) -OPTION (CARES_BUILD_TOOLS "Build tools" ON) -OPTION (CARES_SYMBOL_HIDING "Hide private symbols in shared libraries" OFF) +OPTION (CARES_BUILD_TOOLS "Build tools" ON) +OPTION (CARES_SYMBOL_HIDING "Hide private symbols in shared libraries" OFF) +OPTION (CARES_THREADS "Build with thread-safety support" ON) SET (CARES_RANDOM_FILE "/dev/urandom" CACHE STRING "Suitable File / Device Path for entropy, such as /dev/urandom") @@ -442,6 +443,43 @@ SET (CMAKE_REQUIRED_DEFINITIONS) SET (CMAKE_REQUIRED_LIBRARIES) +################################################################################ +# Threading Support +# +IF (CARES_THREADS) + IF (WIN32) + # Do nothing, always has threads + ELSE () + # Need to prefer pthreads on platforms that may have more threading choices + # (e.g. Solaris) + SET (CMAKE_THREAD_PREFER_PTHREAD TRUE) + FIND_PACKAGE (Threads) + + IF (Threads_FOUND) + # Fix solaris9 bug due to libc having pthread_create() stubs that always fail. CMake + # doesn't realize that the real pthread functions aren't in libc, so sets the pthread + # library CAKE_THREAD_LIBS_INIT variable to blank instead of to the correct "-lpthread". + IF (CMAKE_SYSTEM_NAME STREQUAL "SunOS" AND NOT CMAKE_THREAD_LIBS_INIT) + SET (CMAKE_THREAD_LIBS_INIT "-lpthread") + ENDIF () + + # PThread functions. + CHECK_INCLUDE_FILES (pthread.h HAVE_PTHREAD_H) + CHECK_INCLUDE_FILES (pthread_np.h HAVE_PTHREAD_NP_H) + CARES_EXTRAINCLUDE_IFSET (HAVE_PTHREAD_H pthread.h) + CARES_EXTRAINCLUDE_IFSET (HAVE_PTHREAD_NP_H pthread_np.h) + CHECK_SYMBOL_EXISTS (pthread_init "${CMAKE_EXTRA_INCLUDE_FILES}" HAVE_PTHREAD_INIT) + # Make sure libcares.pc.cmake knows about thread libraries on static builds + LIST (APPEND CARES_DEPENDENT_LIBS ${CMAKE_THREAD_LIBS_INIT}) + ELSE () + MESSAGE (WARNING "Threading support not found, disabling...") + SET (CARES_THREADS OFF) + ENDIF () + ENDIF () +ENDIF () + + + ################################################################################ # recv, recvfrom, send, getnameinfo, gethostname # ARGUMENTS AND RETURN VALUES diff --git a/configure.ac b/configure.ac index 89adece9..25c42b95 100644 --- a/configure.ac +++ b/configure.ac @@ -972,6 +972,33 @@ fi CARES_CHECK_OPTION_NONBLOCKING CARES_CHECK_NONBLOCKING_SOCKET + +dnl ------------ THREADING -------------- +AC_ARG_ENABLE(cares-threads, + AC_HELP_STRING([--disable-cares-threads], [Disable building of thread safety support]), + [ CARES_THREADS=${enableval} ], + [ CARES_THREADS=yes ]) + +dnl windows always supports threads, only check non-windows systems. +if test "${CARES_THREADS}" = "yes" -a "x${ac_cv_native_windows}" != "xyes" ; then + AX_PTHREAD([ ], [ + AC_MSG_WARN([threads requested but not supported]) + CARES_THREADS=no + ]) + + if test "${CARES_THREADS}" = "yes" ; then + AC_CHECK_HEADERS([pthread.h pthread_np.h]) + LIBS="$PTHREAD_LIBS $LIBS" + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + CC="$PTHREAD_CC" + CXX="$PTHREAD_CXX" + fi +fi + +if test "${CARES_THREADS}" = "yes" ; then + AC_DEFINE([CARES_THREADS], [ 1 ], [Threading enabled]) +fi + CARES_CONFIGURE_SYMBOL_HIDING CARES_PRIVATE_LIBS="$LIBS" diff --git a/docs/ares_destroy.3 b/docs/ares_destroy.3 index 26719e65..a7924890 100644 --- a/docs/ares_destroy.3 +++ b/docs/ares_destroy.3 @@ -33,9 +33,13 @@ by the channel. channel, passing a status of \fIARES_EDESTRUCTION\fP. These calls give the callbacks a chance to clean up any state which might have been stored in their arguments. A callback must not add new requests to a channel being destroyed. + +There is no ability to make this function thread-safe. No additional calls +using this channel may be made once this function is called. .SH SEE ALSO .BR ares_init (3), -.BR ares_cancel (3) +.BR ares_cancel (3), +.BR ares_threadsafety (3) .SH AUTHOR Greg Hudson, MIT Information Systems .br diff --git a/docs/ares_dns_mapping.3 b/docs/ares_dns_mapping.3 index c0df8f74..6c2c905a 100644 --- a/docs/ares_dns_mapping.3 +++ b/docs/ares_dns_mapping.3 @@ -153,7 +153,8 @@ with \fIARES_RR_SVCB_PARAMS\fP or \fIARES_RR_HTTPS_PARAMS\fP: .B ARES_SVCB_PARAM_ECH - RESERVED (held for Encrypted ClientHello) .br -.B ARES_SVCB_PARAM_IPV6HINT. IPv6 address hints (RFC 9460 Section 7.3). Datatype: \fIARES_OPT_DATATYPE_INADDR6_LIST\fP +.B ARES_SVCB_PARAM_IPV6HINT +- IPv6 address hints (RFC 9460 Section 7.3). Datatype: \fIARES_OPT_DATATYPE_INADDR6_LIST\fP .br .RE diff --git a/docs/ares_getaddrinfo.3 b/docs/ares_getaddrinfo.3 index 812e1547..27585aa3 100644 --- a/docs/ares_getaddrinfo.3 +++ b/docs/ares_getaddrinfo.3 @@ -91,6 +91,12 @@ Completion or failure of the query may happen immediately, or may happen during a later call to \fIares_process(3)\fP, \fIares_destroy(3)\fP or \fIares_cancel(3)\fP. .PP +If this is called from a thread other than which the main program event loop is +running, care needs to be taken to ensure any file descriptor lists are updated +immediately within the eventloop. When the associated callback is called, +it is called with a channel lock so care must be taken to ensure any processing +is minimal to prevent DNS channel stalls. +.PP The callback argument .I arg is copied from the \fBares_getaddrinfo(3)\fP argument diff --git a/docs/ares_gethostbyaddr.3 b/docs/ares_gethostbyaddr.3 index 6c3b097d..4a250505 100644 --- a/docs/ares_gethostbyaddr.3 +++ b/docs/ares_gethostbyaddr.3 @@ -98,6 +98,12 @@ did not complete successfully, .I hostent will be .BR NULL . +.PP +If this is called from a thread other than which the main program event loop is +running, care needs to be taken to ensure any file descriptor lists are updated +immediately within the eventloop. When the associated callback is called, +it is called with a channel lock so care must be taken to ensure any processing +is minimal to prevent DNS channel stalls. .SH SEE ALSO .BR ares_process (3), .BR ares_gethostbyname (3) diff --git a/docs/ares_gethostbyname.3 b/docs/ares_gethostbyname.3 index 253af632..e6d302d0 100644 --- a/docs/ares_gethostbyname.3 +++ b/docs/ares_gethostbyname.3 @@ -106,6 +106,12 @@ did not complete successfully, .I hostent will be .BR NULL . +.PP +If this is called from a thread other than which the main program event loop is +running, care needs to be taken to ensure any file descriptor lists are updated +immediately within the eventloop. When the associated callback is called, +it is called with a channel lock so care must be taken to ensure any processing +is minimal to prevent DNS channel stalls. .SH SEE ALSO .BR ares_process (3), .BR ares_gethostbyaddr (3) diff --git a/docs/ares_getnameinfo.3 b/docs/ares_getnameinfo.3 index 786c0e0e..c941a68f 100644 --- a/docs/ares_getnameinfo.3 +++ b/docs/ares_getnameinfo.3 @@ -86,6 +86,12 @@ failed, the ares library will invoke \fIcallback\fP. Completion or failure of the query may happen immediately, or may happen during a later call to \fIares_process(3)\fP, \fIares_destroy(3)\fP or \fIares_cancel(3)\fP. .PP +If this is called from a thread other than which the main program event loop is +running, care needs to be taken to ensure any file descriptor lists are updated +immediately within the eventloop. When the associated callback is called, +it is called with a channel lock so care must be taken to ensure any processing +is minimal to prevent DNS channel stalls. +.PP The callback argument .I arg is copied from the diff --git a/docs/ares_init_options.3 b/docs/ares_init_options.3 index 99242598..282670c7 100644 --- a/docs/ares_init_options.3 +++ b/docs/ares_init_options.3 @@ -57,6 +57,19 @@ for name service lookups. If it returns successfully, caller should invoke \fIares_destroy(3)\fP on the handle when the channel is no longer needed. +It is recommended for an application to have at most one ares channel and use +this for all DNS queries for the life of the application. When system +configuration changes, \fIares_reinit(3)\fP can be called to reload the +configuration if necessary. The recommended concurrent query limit is about +32k queries, but remembering that when specifying AF_UNSPEC for +\fBares_getaddrinfo(3)\fP or \fBares_gethostbyname(3)\fP, they may spawn +2 queries internally. The reason for the limit is c-ares does not allow +duplicate DNS query ids (which have a maximum of 64k) to be oustanding at a +given time, and it must randomly search for an available id thus 32k will limit +the number of searches. This limitation should not be a concern for most +implementations and c-ares may implement queuing in future releases to lift this +limitation. + The \fIoptmask\fP parameter generally specifies which fields in the structure pointed to by \fIoptions\fP are set, as follows: .TP 18 @@ -323,7 +336,8 @@ manual page. .BR ares_library_init (3), .BR ares_save_options (3), .BR ares_set_servers (3), -.BR ares_set_sortlist (3) +.BR ares_set_sortlist (3), +.BR ares_threadsafety (3) .SH AUTHOR Greg Hudson, MIT Information Systems .br diff --git a/docs/ares_query.3 b/docs/ares_query.3 index e5eb729f..f87a5648 100644 --- a/docs/ares_query.3 +++ b/docs/ares_query.3 @@ -54,6 +54,12 @@ happen during a later call to or .BR ares_destroy (3). .PP +If this is called from a thread other than which the main program event loop is +running, care needs to be taken to ensure any file descriptor lists are updated +immediately within the eventloop. When the associated callback is called, +it is called with a channel lock so care must be taken to ensure any processing +is minimal to prevent DNS channel stalls. +.PP The callback argument .I arg is copied from the diff --git a/docs/ares_reinit.3 b/docs/ares_reinit.3 index 1b99fec7..0b037127 100644 --- a/docs/ares_reinit.3 +++ b/docs/ares_reinit.3 @@ -19,11 +19,11 @@ never override user-provided settings such as provided via Any existing queries will be automatically requeued if the server they are currently assigned to is removed from the system configuration. -It is important to remember that c-ares is not currently thread-aware and this -function cannot be called while any other c-ares functions are being called. -This function may also cause additional file descriptors to be created if -server configuration has changed, so care needs to be taken to ensure any -event loop is updated with new file descriptors. +This function may cause additional file descriptors to be created, and existing +ones to be destroyed if server configuration has changed. If this is called from +a thread other than which the main program event loop is running, care needs to +be taken to ensure any file descriptor lists are updated immediately within +the eventloop. .SH RETURN VALUES \fIares_reinit(3)\fP can return any of the following values: @@ -45,6 +45,7 @@ This function was first introduced in c-ares version 1.22.0. .BR ares_destroy (3), .BR ares_dup (3), .BR ares_library_init (3), -.BR ares_set_servers (3) +.BR ares_set_servers (3), +.BR ares_threadsafety (3) .SH AUTHOR Copyright (C) 2023 The c-ares project and its members. diff --git a/docs/ares_search.3 b/docs/ares_search.3 index a1ee65aa..e4db64db 100644 --- a/docs/ares_search.3 +++ b/docs/ares_search.3 @@ -57,6 +57,12 @@ may happen during a later call to or .BR ares_destroy (3). .PP +If this is called from a thread other than which the main program event loop is +running, care needs to be taken to ensure any file descriptor lists are updated +immediately within the eventloop. When the associated callback is called, +it is called with a channel lock so care must be taken to ensure any processing +is minimal to prevent DNS channel stalls. +.PP The callback argument .I arg is copied from the diff --git a/docs/ares_send.3 b/docs/ares_send.3 index 5de24fc4..83ddb7ee 100644 --- a/docs/ares_send.3 +++ b/docs/ares_send.3 @@ -49,6 +49,12 @@ happen during a later call to or .BR ares_destroy (3). .PP +If this is called from a thread other than which the main program event loop is +running, care needs to be taken to ensure any file descriptor lists are updated +immediately within the eventloop. When the associated callback is called, +it is called with a channel lock so care must be taken to ensure any processing +is minimal to prevent DNS channel stalls. +.PP The callback argument .I arg is copied from the diff --git a/docs/ares_threadsafety.3 b/docs/ares_threadsafety.3 new file mode 100644 index 00000000..8f61a876 --- /dev/null +++ b/docs/ares_threadsafety.3 @@ -0,0 +1,43 @@ +.\" +.\" SPDX-License-Identifier: MIT +.\" +.TH ARES_REINIT 3 "26 November 2023" +.SH NAME +ares_threadsafety \- Query if c-ares was built with thread-safety +.SH SYNOPSIS +.nf +#include + +ares_bool_t ares_threadsafety(void); +.fi +.SH DESCRIPTION +The \fBares_threadsafety(3)\fP function returns if the library was built with +thread safety enabled or not. + +As of c-ares 1.23.0, this simply means that every public function which +references an \fIares_channel_t\fP object will lock the channel on entry and +release the lock on exit of the function. This will simply prevent concurrent +thread access to the channel, thus ensuring no corruption can occur. Future +versions will likely implement more threading-specific features. + +.SH RETURN VALUES +\fIares_threadsafety(3)\fP can return any of the following values: +.TP 14 +.B ARES_TRUE +Built with thread safety. +.TP 14 +.B ARES_FALSE +Built without thread safety +.TP 14 + +.SH AVAILABILITY +This function was first introduced in c-ares version 1.23.0. +.SH SEE ALSO +.BR ares_init (3), +.BR ares_init_options (3), +.BR ares_destroy (3), +.BR ares_dup (3), +.BR ares_library_init (3), +.BR ares_set_servers (3) +.SH AUTHOR +Copyright (C) 2023 The c-ares project and its members. diff --git a/include/ares.h b/include/ares.h index 619eec92..7e2e1899 100644 --- a/include/ares.h +++ b/include/ares.h @@ -741,6 +741,11 @@ CARES_EXTERN const char *ares_inet_ntop(int af, const void *src, char *dst, CARES_EXTERN int ares_inet_pton(int af, const char *src, void *dst); +/*! Whether or not the c-ares library was built with threadsafety + * + * \return ARES_TRUE if built with threadsafety, ARES_FALSE if not + */ +CARES_EXTERN ares_status_t ares_threadsafety(void); #ifdef __cplusplus } diff --git a/src/lib/CMakeLists.txt b/src/lib/CMakeLists.txt index 7cb37f16..373e1fbd 100644 --- a/src/lib/CMakeLists.txt +++ b/src/lib/CMakeLists.txt @@ -54,7 +54,10 @@ IF (CARES_SHARED) TARGET_COMPILE_DEFINITIONS (${PROJECT_NAME} PRIVATE HAVE_CONFIG_H=1 CARES_BUILDING_LIBRARY) - TARGET_LINK_LIBRARIES (${PROJECT_NAME} PUBLIC ${CARES_DEPENDENT_LIBS}) + TARGET_LINK_LIBRARIES (${PROJECT_NAME} + PUBLIC ${CARES_DEPENDENT_LIBS} + PRIVATE ${CMAKE_THREAD_LIBS_INIT} + ) IF (CARES_INSTALL) INSTALL (TARGETS ${PROJECT_NAME} diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 823bb928..8324a355 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -16,6 +16,7 @@ CSOURCES = ares__addrinfo2hostent.c \ ares__slist.c \ ares__socket.c \ ares__sortaddrinfo.c \ + ares__threads.c \ ares__timeval.c \ ares_android.c \ ares_cancel.c \ diff --git a/src/lib/ares__threads.c b/src/lib/ares__threads.c new file mode 100644 index 00000000..e2035ac7 --- /dev/null +++ b/src/lib/ares__threads.c @@ -0,0 +1,184 @@ +/* MIT License + * + * Copyright (c) 2023 Brad House + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + * SPDX-License-Identifier: MIT + */ +#include "ares_setup.h" +#include "ares.h" +#include "ares_private.h" + +#ifdef CARES_THREADS +# ifdef _WIN32 + +struct ares__thread_mutex { + CRITICAL_SECTION mutex; +}; + +static ares__thread_mutex_t *ares__thread_mutex_create(void) +{ + ares__thread_mutex_t *mut = ares_malloc_zero(sizeof(*mut)); + if (mut == NULL) + return NULL; + + InitializeCriticalSection(&mut->mutex); + return mut; +} + +static void ares__thread_mutex_destroy(ares__thread_mutex_t *mut) +{ + if (mut == NULL) + return; + DeleteCriticalSection(&mut->mutex); + ares_free(mut); +} + +static void ares__thread_mutex_lock(ares__thread_mutex_t *mut) +{ + if (mut == NULL) + return; + EnterCriticalSection(&mut->mutex); +} + +static void ares__thread_mutex_unlock(ares__thread_mutex_t *mut) +{ + if (mut == NULL) + return; + LeaveCriticalSection(&mut->mutex); +} + +# else +# include + +struct ares__thread_mutex { + pthread_mutex_t mutex; +}; + +static ares__thread_mutex_t *ares__thread_mutex_create(void) +{ + pthread_mutexattr_t attr; + ares__thread_mutex_t *mut = ares_malloc_zero(sizeof(*mut)); + if (mut == NULL) + return NULL; + + if (pthread_mutexattr_init(&attr) != 0) { + ares_free(mut); + return NULL; + } + + if (pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE) != 0) { + goto fail; + } + + if (pthread_mutex_init(&mut->mutex, &attr) != 0) { + goto fail; + } + + pthread_mutexattr_destroy(&attr); + return mut; + +fail: + pthread_mutexattr_destroy(&attr); + ares_free(mut); + return NULL; +} + +static void ares__thread_mutex_destroy(ares__thread_mutex_t *mut) +{ + if (mut == NULL) + return; + pthread_mutex_destroy(&mut->mutex); + ares_free(mut); +} + +static void ares__thread_mutex_lock(ares__thread_mutex_t *mut) +{ + if (mut == NULL) + return; + pthread_mutex_lock(&mut->mutex); +} + +static void ares__thread_mutex_unlock(ares__thread_mutex_t *mut) +{ + if (mut == NULL) + return; + pthread_mutex_unlock(&mut->mutex); +} +#endif + +ares_status_t ares__channel_threading_init(ares_channel_t *channel) +{ + channel->lock = ares__thread_mutex_create(); + if (channel->lock == NULL) + return ARES_ENOMEM; + return ARES_SUCCESS; +} + +void ares__channel_threading_destroy(ares_channel_t *channel) +{ + ares__thread_mutex_destroy(channel->lock); + channel->lock = NULL; +} + +void ares__channel_lock(ares_channel_t *channel) +{ + ares__thread_mutex_lock(channel->lock); +} + +void ares__channel_unlock(ares_channel_t *channel) +{ + ares__thread_mutex_unlock(channel->lock); +} + +ares_status_t ares_threadsafety(void) +{ + return ARES_TRUE; +} + +#else +/* NoOp */ +ares_status_t ares__channel_threading_init(ares_channel_t *channel) +{ + (void)channel; + return ARES_SUCCESS; +} + +void ares__channel_threading_destroy(ares_channel_t *channel) +{ + (void)channel; +} + +void ares__channel_lock(ares_channel_t *channel) +{ + (void)channel; +} + +void ares__channel_unlock(ares_channel_t *channel) +{ + (void)channel; +} + +ares_status_t ares_threadsafety(void) +{ + return ARES_FALSE; +} +#endif diff --git a/src/lib/ares_cancel.c b/src/lib/ares_cancel.c index ac32b7d8..3abbea32 100644 --- a/src/lib/ares_cancel.c +++ b/src/lib/ares_cancel.c @@ -37,6 +37,11 @@ */ void ares_cancel(ares_channel_t *channel) { + if (channel == NULL) + return; + + ares__channel_lock(channel); + if (ares__llist_len(channel->all_queries) > 0) { ares__llist_node_t *node = NULL; ares__llist_node_t *next = NULL; @@ -52,7 +57,7 @@ void ares_cancel(ares_channel_t *channel) * can't report to caller */ if (channel->all_queries == NULL) { channel->all_queries = list_copy; - return; + goto done; } node = ares__llist_node_first(list_copy); @@ -79,4 +84,6 @@ void ares_cancel(ares_channel_t *channel) ares__llist_destroy(list_copy); } +done: + ares__channel_unlock(channel); } diff --git a/src/lib/ares_config.h.cmake b/src/lib/ares_config.h.cmake index 9f35ed8b..84880695 100644 --- a/src/lib/ares_config.h.cmake +++ b/src/lib/ares_config.h.cmake @@ -449,3 +449,15 @@ /* Type to use in place of in_addr_t when system does not provide it. */ #undef in_addr_t +/* Define to 1 if you have the pthread.h header file. */ +#cmakedefine HAVE_PTHREAD_H + +/* Define to 1 if you have the pthread_np.h header file. */ +#cmakedefine HAVE_PTHREAD_NP_H + +/* Define to 1 if threads are enabled */ +#cmakedefine CARES_THREADS + +/* Define to 1 if pthread_init() exists */ +#cmakedefine HAVE_PTHREAD_INIT + diff --git a/src/lib/ares_destroy.c b/src/lib/ares_destroy.c index 68efb2b2..d7cfb3bb 100644 --- a/src/lib/ares_destroy.c +++ b/src/lib/ares_destroy.c @@ -41,6 +41,9 @@ void ares_destroy(ares_channel_t *channel) return; } + /* Lock because callbacks will be triggered */ + ares__channel_lock(channel); + /* Destroy all queries */ node = ares__llist_node_first(channel->all_queries); while (node != NULL) { @@ -69,6 +72,9 @@ void ares_destroy(ares_channel_t *channel) assert(ares__htable_asvp_num_keys(channel->connnode_by_socket) == 0); #endif + /* No more callbacks will be triggered after this point, unlock */ + ares__channel_unlock(channel); + if (channel->domains) { for (i = 0; i < channel->ndomains; i++) { ares_free(channel->domains[i]); @@ -91,6 +97,8 @@ void ares_destroy(ares_channel_t *channel) ares__qcache_destroy(channel->qcache); + ares__channel_threading_destroy(channel); + ares_free(channel); } diff --git a/src/lib/ares_fds.c b/src/lib/ares_fds.c index a402b31f..e80605a1 100644 --- a/src/lib/ares_fds.c +++ b/src/lib/ares_fds.c @@ -34,9 +34,15 @@ int ares_fds(ares_channel_t *channel, fd_set *read_fds, fd_set *write_fds) { ares_socket_t nfds; ares__slist_node_t *snode; - /* Are there any active queries? */ - size_t active_queries = ares__llist_len(channel->all_queries); + size_t active_queries; + + if (channel == NULL || read_fds == NULL || write_fds == NULL) + return 0; + + ares__channel_lock(channel); + + active_queries = ares__llist_len(channel->all_queries); nfds = 0; for (snode = ares__slist_node_first(channel->servers); snode != NULL; @@ -71,5 +77,6 @@ int ares_fds(ares_channel_t *channel, fd_set *read_fds, fd_set *write_fds) } } + ares__channel_unlock(channel); return (int)nfds; } diff --git a/src/lib/ares_getaddrinfo.c b/src/lib/ares_getaddrinfo.c index fa9212db..1e95bee6 100644 --- a/src/lib/ares_getaddrinfo.c +++ b/src/lib/ares_getaddrinfo.c @@ -532,10 +532,10 @@ static void host_callback(void *arg, int status, int timeouts, /* at this point we keep on waiting for the next query to finish */ } -void ares_getaddrinfo(ares_channel_t *channel, const char *name, - const char *service, - const struct ares_addrinfo_hints *hints, - ares_addrinfo_callback callback, void *arg) +static void ares_getaddrinfo_int(ares_channel_t *channel, const char *name, + const char *service, + const struct ares_addrinfo_hints *hints, + ares_addrinfo_callback callback, void *arg) { struct host_query *hquery; unsigned short port = 0; @@ -668,6 +668,18 @@ void ares_getaddrinfo(ares_channel_t *channel, const char *name, next_lookup(hquery, ARES_ECONNREFUSED /* initial error code */); } +void ares_getaddrinfo(ares_channel_t *channel, const char *name, + const char *service, + const struct ares_addrinfo_hints *hints, + ares_addrinfo_callback callback, void *arg) +{ + if (channel == NULL) + return; + ares__channel_lock(channel); + ares_getaddrinfo_int(channel, name, service, hints, callback, arg); + ares__channel_unlock(channel); +} + static ares_bool_t next_dns_lookup(struct host_query *hquery) { char *s = NULL; diff --git a/src/lib/ares_gethostbyaddr.c b/src/lib/ares_gethostbyaddr.c index 4248104b..bd8e715e 100644 --- a/src/lib/ares_gethostbyaddr.c +++ b/src/lib/ares_gethostbyaddr.c @@ -68,8 +68,9 @@ static ares_status_t file_lookup(ares_channel_t *channel, const struct ares_addr *addr, struct hostent **host); -void ares_gethostbyaddr(ares_channel_t *channel, const void *addr, int addrlen, - int family, ares_host_callback callback, void *arg) +static void ares_gethostbyaddr_int(ares_channel_t *channel, const void *addr, + int addrlen, int family, + ares_host_callback callback, void *arg) { struct addr_query *aquery; @@ -110,6 +111,16 @@ void ares_gethostbyaddr(ares_channel_t *channel, const void *addr, int addrlen, next_lookup(aquery); } +void ares_gethostbyaddr(ares_channel_t *channel, const void *addr, int addrlen, + int family, ares_host_callback callback, void *arg) +{ + if (channel == NULL) + return; + ares__channel_lock(channel); + ares_gethostbyaddr_int(channel, addr, addrlen, family, callback, arg); + ares__channel_unlock(channel); +} + static void next_lookup(struct addr_query *aquery) { const char *p; diff --git a/src/lib/ares_gethostbyname.c b/src/lib/ares_gethostbyname.c index 140dffdb..a9fe4e8e 100644 --- a/src/lib/ares_gethostbyname.c +++ b/src/lib/ares_gethostbyname.c @@ -120,6 +120,8 @@ void ares_gethostbyname(ares_channel_t *channel, const char *name, int family, ghbn_arg->arg = arg; ghbn_arg->channel = channel; + /* NOTE: ares_getaddrinfo() locks the channel, we don't use the channel + * outside so no need to lock */ ares_getaddrinfo(channel, name, NULL, &hints, ares_gethostbyname_callback, ghbn_arg); } @@ -263,8 +265,10 @@ done: /* I really have no idea why this is exposed as a public function, but since * it is, we can't kill this legacy function. */ -int ares_gethostbyname_file(ares_channel_t *channel, const char *name, - int family, struct hostent **host) +static ares_status_t ares_gethostbyname_file_int(ares_channel_t *channel, + const char *name, + int family, + struct hostent **host) { const ares_hosts_entry_t *entry; ares_status_t status; @@ -302,8 +306,22 @@ done: * as permissions errors reading /etc/hosts or a malformed /etc/hosts */ if (status != ARES_SUCCESS && status != ARES_ENOMEM && ares__is_localhost(name)) { - return (int)ares__hostent_localhost(name, family, host); + return ares__hostent_localhost(name, family, host); + } + + return status; +} + +int ares_gethostbyname_file(ares_channel_t *channel, const char *name, + int family, struct hostent **host) +{ + ares_status_t status; + if (channel == NULL) { + return ARES_ENOTFOUND; } + ares__channel_lock(channel); + status = ares_gethostbyname_file_int(channel, name, family, host); + ares__channel_unlock(channel); return (int)status; } diff --git a/src/lib/ares_getnameinfo.c b/src/lib/ares_getnameinfo.c index 1ec09203..2adcc56a 100644 --- a/src/lib/ares_getnameinfo.c +++ b/src/lib/ares_getnameinfo.c @@ -86,9 +86,10 @@ static void append_scopeid(const struct sockaddr_in6 *addr6, #endif STATIC_TESTABLE char *ares_striendstr(const char *s1, const char *s2); -void ares_getnameinfo(ares_channel_t *channel, const struct sockaddr *sa, - ares_socklen_t salen, int flags_int, - ares_nameinfo_callback callback, void *arg) +static void ares_getnameinfo_int(ares_channel_t *channel, + const struct sockaddr *sa, + ares_socklen_t salen, int flags_int, + ares_nameinfo_callback callback, void *arg) { const struct sockaddr_in *addr = NULL; const struct sockaddr_in6 *addr6 = NULL; @@ -185,6 +186,18 @@ void ares_getnameinfo(ares_channel_t *channel, const struct sockaddr *sa, } } +void ares_getnameinfo(ares_channel_t *channel, const struct sockaddr *sa, + ares_socklen_t salen, int flags_int, + ares_nameinfo_callback callback, void *arg) +{ + if (channel == NULL) + return; + + ares__channel_lock(channel); + ares_getnameinfo_int(channel, sa, salen, flags_int, callback, arg); + ares__channel_unlock(channel); +} + static void nameinfo_callback(void *arg, int status, int timeouts, struct hostent *host) { diff --git a/src/lib/ares_getsock.c b/src/lib/ares_getsock.c index 93a4a7f3..0353bad6 100644 --- a/src/lib/ares_getsock.c +++ b/src/lib/ares_getsock.c @@ -38,12 +38,16 @@ int ares_getsock(ares_channel_t *channel, ares_socket_t *socks, unsigned int setbits = 0xffffffff; /* Are there any active queries? */ - size_t active_queries = ares__llist_len(channel->all_queries); + size_t active_queries; - if (numsocks <= 0) { + if (channel == NULL || numsocks <= 0) { return 0; } + ares__channel_lock(channel); + + active_queries = ares__llist_len(channel->all_queries); + for (snode = ares__slist_node_first(channel->servers); snode != NULL; snode = ares__slist_node_next(snode)) { struct server_state *server = ares__slist_node_val(snode); @@ -78,5 +82,7 @@ int ares_getsock(ares_channel_t *channel, ares_socket_t *socks, sockindex++; } } + + ares__channel_unlock(channel); return (int)bitmap; } diff --git a/src/lib/ares_init.c b/src/lib/ares_init.c index e0eb5a25..703194ee 100644 --- a/src/lib/ares_init.c +++ b/src/lib/ares_init.c @@ -302,6 +302,11 @@ int ares_init_options(ares_channel_t **channelptr, return ARES_ENOMEM; } + status = ares__channel_threading_init(channel); + if (status != ARES_SUCCESS) { + goto done; + } + /* Generate random key */ channel->rand_state = ares__init_rand_state(); if (channel->rand_state == NULL) { @@ -402,6 +407,8 @@ ares_status_t ares_reinit(ares_channel_t *channel) return ARES_EFORMERR; } + ares__channel_lock(channel); + status = ares__init_by_sysconfig(channel); if (status != ARES_SUCCESS) { DEBUGF(fprintf(stderr, "Error: init_by_sysconfig failed: %s\n", @@ -413,6 +420,8 @@ ares_status_t ares_reinit(ares_channel_t *channel) ares__qcache_flush(channel->qcache); } + ares__channel_unlock(channel); + return status; } @@ -425,14 +434,19 @@ int ares_dup(ares_channel_t **dest, ares_channel_t *src) ares_status_t rc; int optmask; + if (dest == NULL || src == NULL) { + return ARES_EFORMERR; + } + *dest = NULL; /* in case of failure return NULL explicitly */ + ares__channel_lock(src); /* First get the options supported by the old ares_save_options() function, which is most of them */ rc = (ares_status_t)ares_save_options(src, &opts, &optmask); if (rc != ARES_SUCCESS) { ares_destroy_options(&opts); - return (int)rc; + goto done; } /* Then create the new channel with those options */ @@ -442,7 +456,7 @@ int ares_dup(ares_channel_t **dest, ares_channel_t *src) ares_destroy_options(&opts); if (rc != ARES_SUCCESS) { - return (int)rc; + goto done; } /* Now clone the options that ares_save_options() doesn't support, but are @@ -473,7 +487,7 @@ int ares_dup(ares_channel_t **dest, ares_channel_t *src) if (rc != ARES_SUCCESS) { ares_destroy(*dest); *dest = NULL; - return (int)rc; + goto done; } rc = (ares_status_t)ares_set_servers_ports(*dest, servers); @@ -481,30 +495,46 @@ int ares_dup(ares_channel_t **dest, ares_channel_t *src) if (rc != ARES_SUCCESS) { ares_destroy(*dest); *dest = NULL; - return (int)rc; + goto done; } } - return ARES_SUCCESS; /* everything went fine */ + rc = ARES_SUCCESS; +done: + ares__channel_unlock(src); + return (int)rc; /* everything went fine */ } void ares_set_local_ip4(ares_channel_t *channel, unsigned int local_ip) { + if (channel == NULL) + return; + ares__channel_lock(channel); channel->local_ip4 = local_ip; + ares__channel_unlock(channel); } /* local_ip6 should be 16 bytes in length */ void ares_set_local_ip6(ares_channel_t *channel, const unsigned char *local_ip6) { + if (channel == NULL) + return; + ares__channel_lock(channel); memcpy(&channel->local_ip6, local_ip6, sizeof(channel->local_ip6)); + ares__channel_unlock(channel); } /* local_dev_name should be null terminated. */ void ares_set_local_dev(ares_channel_t *channel, const char *local_dev_name) { + if (channel == NULL) + return; + + ares__channel_lock(channel); ares_strcpy(channel->local_dev_name, local_dev_name, sizeof(channel->local_dev_name)); channel->local_dev_name[sizeof(channel->local_dev_name) - 1] = 0; + ares__channel_unlock(channel); } int ares_set_sortlist(ares_channel_t *channel, const char *sortstr) @@ -516,6 +546,7 @@ int ares_set_sortlist(ares_channel_t *channel, const char *sortstr) if (!channel) { return ARES_ENODATA; } + ares__channel_lock(channel); status = ares__parse_sortlist(&sortlist, &nsort, sortstr); if (status == ARES_SUCCESS && sortlist) { @@ -528,5 +559,6 @@ int ares_set_sortlist(ares_channel_t *channel, const char *sortstr) /* Save sortlist as if it was passed in as an option */ channel->optmask |= ARES_OPT_SORTLIST; } + ares__channel_unlock(channel); return (int)status; } diff --git a/src/lib/ares_private.h b/src/lib/ares_private.h index 9ea59b57..a7de8ca7 100644 --- a/src/lib/ares_private.h +++ b/src/lib/ares_private.h @@ -249,6 +249,9 @@ typedef struct ares__qcache ares__qcache_t; struct ares_hosts_file; typedef struct ares_hosts_file ares_hosts_file_t; +struct ares__thread_mutex; +typedef struct ares__thread_mutex ares__thread_mutex_t; + struct ares_channeldata { /* Configuration data */ unsigned int flags; @@ -277,6 +280,9 @@ struct ares_channeldata { unsigned int local_ip4; unsigned char local_ip6[16]; + /* Thread safety lock */ + ares__thread_mutex_t *lock; + /* Server addresses and communications state. Sorted by least consecutive * failures, followed by the configuration order if failures are equal. */ ares__slist_t *servers; @@ -577,6 +583,11 @@ ares_status_t ares_qcache_fetch(ares_channel_t *channel, const unsigned char *qbuf, size_t qlen, unsigned char **abuf, size_t *alen); +ares_status_t ares__channel_threading_init(ares_channel_t *channel); +void ares__channel_threading_destroy(ares_channel_t *channel); +void ares__channel_lock(ares_channel_t *channel); +void ares__channel_unlock(ares_channel_t *channel); + #ifdef _MSC_VER typedef __int64 ares_int64_t; typedef unsigned __int64 ares_uint64_t; diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c index f80ef8af..fa0ff331 100644 --- a/src/lib/ares_process.c +++ b/src/lib/ares_process.c @@ -140,12 +140,20 @@ static void processfds(ares_channel_t *channel, fd_set *read_fds, ares_socket_t read_fd, fd_set *write_fds, ares_socket_t write_fd) { - struct timeval now = ares__tvnow(); + struct timeval now; + if (channel == NULL) + return; + + ares__channel_lock(channel); + + now = ares__tvnow(); read_packets(channel, read_fds, read_fd, &now); process_timeouts(channel, &now); /* Write last as the other 2 operations might have triggered writes */ write_tcp_data(channel, write_fds, write_fd); + + ares__channel_unlock(channel); } /* Something interesting happened on the wire, or there was a timeout. diff --git a/src/lib/ares_query.c b/src/lib/ares_query.c index 54d5be95..50ce3daa 100644 --- a/src/lib/ares_query.c +++ b/src/lib/ares_query.c @@ -88,7 +88,11 @@ ares_status_t ares_query_qid(ares_channel_t *channel, const char *name, void ares_query(ares_channel_t *channel, const char *name, int dnsclass, int type, ares_callback callback, void *arg) { + if (channel == NULL) + return; + ares__channel_lock(channel); ares_query_qid(channel, name, dnsclass, type, callback, arg, NULL); + ares__channel_unlock(channel); } static void qcallback(void *arg, int status, int timeouts, unsigned char *abuf, diff --git a/src/lib/ares_search.c b/src/lib/ares_search.c index 216232e9..ccb3e95a 100644 --- a/src/lib/ares_search.c +++ b/src/lib/ares_search.c @@ -57,8 +57,8 @@ static void search_callback(void *arg, int status, int timeouts, static void end_squery(struct search_query *squery, ares_status_t status, unsigned char *abuf, size_t alen); -void ares_search(ares_channel_t *channel, const char *name, int dnsclass, - int type, ares_callback callback, void *arg) +static void ares_search_int(ares_channel_t *channel, const char *name, int dnsclass, + int type, ares_callback callback, void *arg) { struct search_query *squery; char *s; @@ -157,6 +157,16 @@ void ares_search(ares_channel_t *channel, const char *name, int dnsclass, } } +void ares_search(ares_channel_t *channel, const char *name, int dnsclass, + int type, ares_callback callback, void *arg) +{ + if (channel == NULL) + return; + ares__channel_lock(channel); + ares_search_int(channel, name, dnsclass, type, callback, arg); + ares__channel_unlock(channel); +} + static void search_callback(void *arg, int status, int timeouts, unsigned char *abuf, int alen) { diff --git a/src/lib/ares_send.c b/src/lib/ares_send.c index 7c450069..07f11f03 100644 --- a/src/lib/ares_send.c +++ b/src/lib/ares_send.c @@ -153,5 +153,12 @@ ares_status_t ares_send_ex(ares_channel_t *channel, const unsigned char *qbuf, void ares_send(ares_channel_t *channel, const unsigned char *qbuf, int qlen, ares_callback callback, void *arg) { + if (channel == NULL) + return; + + ares__channel_lock(channel); + ares_send_ex(channel, qbuf, (size_t)qlen, callback, arg, NULL); + + ares__channel_unlock(channel); } diff --git a/src/lib/ares_update_servers.c b/src/lib/ares_update_servers.c index 9ed24601..93c06335 100644 --- a/src/lib/ares_update_servers.c +++ b/src/lib/ares_update_servers.c @@ -535,6 +535,8 @@ ares_status_t ares__servers_update(ares_channel_t *channel, return ARES_EFORMERR; } + ares__channel_lock(channel); + /* NOTE: a NULL or zero entry server list is considered valid due to * real-world people needing support for this for their test harnesses */ @@ -589,6 +591,7 @@ ares_status_t ares__servers_update(ares_channel_t *channel, status = ARES_SUCCESS; done: + ares__channel_unlock(channel); return status; } @@ -741,10 +744,12 @@ int ares_get_servers(ares_channel_t *channel, struct ares_addr_node **servers) ares_status_t status = ARES_SUCCESS; ares__slist_node_t *node; - if (!channel) { + if (channel == NULL) { return ARES_ENODATA; } + ares__channel_lock(channel); + for (node = ares__slist_node_first(channel->servers); node != NULL; node = ares__slist_node_next(node)) { const struct server_state *server = ares__slist_node_val(node); @@ -780,6 +785,8 @@ int ares_get_servers(ares_channel_t *channel, struct ares_addr_node **servers) *servers = srvr_head; + ares__channel_unlock(channel); + return (int)status; } @@ -792,10 +799,12 @@ int ares_get_servers_ports(ares_channel_t *channel, ares_status_t status = ARES_SUCCESS; ares__slist_node_t *node; - if (!channel) { + if (channel == NULL) { return ARES_ENODATA; } + ares__channel_lock(channel); + for (node = ares__slist_node_first(channel->servers); node != NULL; node = ares__slist_node_next(node)) { const struct server_state *server = ares__slist_node_val(node); @@ -834,6 +843,7 @@ int ares_get_servers_ports(ares_channel_t *channel, *servers = srvr_head; + ares__channel_unlock(channel); return (int)status; } @@ -852,6 +862,7 @@ int ares_set_servers(ares_channel_t *channel, return (int)status; } + /* NOTE: lock is in ares__servers_update() */ status = ares__servers_update(channel, slist, ARES_TRUE); ares__llist_destroy(slist); @@ -874,6 +885,7 @@ int ares_set_servers_ports(ares_channel_t *channel, return (int)status; } + /* NOTE: lock is in ares__servers_update() */ status = ares__servers_update(channel, slist, ARES_TRUE); ares__llist_destroy(slist); @@ -903,6 +915,8 @@ static ares_status_t set_servers_csv(ares_channel_t *channel, const char *_csv, return ARES_ENODATA; } + /* NOTE: lock is in ares__servers_update() */ + i = ares_strlen(_csv); if (i == 0) { /* blank all servers */ @@ -1034,10 +1048,12 @@ out: int ares_set_servers_csv(ares_channel_t *channel, const char *_csv) { + /* NOTE: lock is in ares__servers_update() */ return (int)set_servers_csv(channel, _csv, ARES_FALSE); } int ares_set_servers_ports_csv(ares_channel_t *channel, const char *_csv) { + /* NOTE: lock is in ares__servers_update() */ return (int)set_servers_csv(channel, _csv, ARES_TRUE); } diff --git a/src/lib/config-win32.h b/src/lib/config-win32.h index da7b67d6..2701f173 100644 --- a/src/lib/config-win32.h +++ b/src/lib/config-win32.h @@ -230,6 +230,9 @@ # undef HAVE_WS2TCPIP_H #endif +/* Threading support enabled */ +#define CARES_THREADS 1 + /* ---------------------------------------------------------------- */ /* TYPEDEF REPLACEMENTS */ /* ---------------------------------------------------------------- */ diff --git a/test/Makefile.am b/test/Makefile.am index 1e03d823..01a2fd85 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -59,12 +59,12 @@ arestest_LDADD = libgmock.la $(ARES_BLD_DIR)/src/lib/libcares.la $(PTHREAD_LIBS) include $(top_srcdir)/aminclude_static.am aresfuzz_SOURCES = $(FUZZSOURCES) -aresfuzz_LDADD = $(ARES_BLD_DIR)/src/lib/libcares.la $(CODE_COVERAGE_LIBS) +aresfuzz_LDADD = $(ARES_BLD_DIR)/src/lib/libcares.la $(PTHREAD_LIBS) $(CODE_COVERAGE_LIBS) aresfuzzname_SOURCES = $(FUZZNAMESOURCES) -aresfuzzname_LDADD = $(ARES_BLD_DIR)/src/lib/libcares.la $(CODE_COVERAGE_LIBS) +aresfuzzname_LDADD = $(ARES_BLD_DIR)/src/lib/libcares.la $(PTHREAD_LIBS) $(CODE_COVERAGE_LIBS) dnsdump_SOURCES = $(DUMPSOURCES) -dnsdump_LDADD = $(ARES_BLD_DIR)/src/lib/libcares.la $(CODE_COVERAGE_LIBS) +dnsdump_LDADD = $(ARES_BLD_DIR)/src/lib/libcares.la $(PTHREAD_LIBS) $(CODE_COVERAGE_LIBS) test: check diff --git a/test/configure.ac b/test/configure.ac index ea76bbd1..daf773b6 100644 --- a/test/configure.ac +++ b/test/configure.ac @@ -34,7 +34,16 @@ AC_PROG_CXX AX_CXX_COMPILE_STDCXX_11([noext],[mandatory]) LT_INIT AC_SUBST(LIBTOOL_DEPS) -AX_PTHREAD +AX_PTHREAD([ CARES_THREADS=yes ], [ CARES_THREADS=no ]) + +if test "${CARES_THREADS}" = "yes" ; then + LIBS="$PTHREAD_LIBS $LIBS" + CFLAGS="$CFLAGS $PTHREAD_CFLAGS" + CXXFLAGS="$CXXFLAGS $PTHREAD_CFLAGS" + CC="$PTHREAD_CC" + CXX="$PTHREAD_CXX" +fi + AX_CODE_COVERAGE AX_CHECK_USER_NAMESPACE AX_CHECK_UTS_NAMESPACE