diff --git a/docs/libcurl/libcurl-env-dbg.md b/docs/libcurl/libcurl-env-dbg.md index d142e94410..3fcc1935d5 100644 --- a/docs/libcurl/libcurl-env-dbg.md +++ b/docs/libcurl/libcurl-env-dbg.md @@ -83,11 +83,6 @@ When built with c-ares for name resolving, setting this environment variable to `[IP:port]` makes libcurl use that DNS server instead of the system default. This is used by the curl test suite. -## `CURL_DNS_DELAY_MS` - -Delay the DNS resolve by this many milliseconds. This is used in the test -suite to check proper handling of CURLOPT_CONNECTTIMEOUT(3). - ## `CURL_FTP_PWD_STOP` When set, the first transfer - when using ftp: - returns before sending diff --git a/lib/asyn-thrdd.c b/lib/asyn-thrdd.c index ca6830a0be..2edef32f7b 100644 --- a/lib/asyn-thrdd.c +++ b/lib/asyn-thrdd.c @@ -199,14 +199,6 @@ err_exit: return NULL; } -static void async_thrd_cleanup(void *arg) -{ - struct async_thrdd_addr_ctx *addr_ctx = arg; - - Curl_thread_disable_cancel(); - addr_ctx_unlink(&addr_ctx, NULL); -} - #ifdef HAVE_GETADDRINFO /* @@ -220,15 +212,6 @@ static CURL_THREAD_RETURN_T CURL_STDCALL getaddrinfo_thread(void *arg) struct async_thrdd_addr_ctx *addr_ctx = arg; bool do_abort; -/* clang complains about empty statements and the pthread_cleanup* macros - * are pretty ill defined. */ -#if defined(__clang__) -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wextra-semi-stmt" -#endif - - Curl_thread_push_cleanup(async_thrd_cleanup, addr_ctx); - Curl_mutex_acquire(&addr_ctx->mutx); do_abort = addr_ctx->do_abort; Curl_mutex_release(&addr_ctx->mutx); @@ -237,9 +220,6 @@ static CURL_THREAD_RETURN_T CURL_STDCALL getaddrinfo_thread(void *arg) char service[12]; int rc; -#ifdef DEBUGBUILD - Curl_resolve_test_delay(); -#endif msnprintf(service, sizeof(service), "%d", addr_ctx->port); rc = Curl_getaddrinfo_ex(addr_ctx->hostname, service, @@ -274,11 +254,6 @@ static CURL_THREAD_RETURN_T CURL_STDCALL getaddrinfo_thread(void *arg) } - Curl_thread_pop_cleanup(); -#if defined(__clang__) -#pragma clang diagnostic pop -#endif - addr_ctx_unlink(&addr_ctx, NULL); return 0; } @@ -293,24 +268,11 @@ static CURL_THREAD_RETURN_T CURL_STDCALL gethostbyname_thread(void *arg) struct async_thrdd_addr_ctx *addr_ctx = arg; bool do_abort; -/* clang complains about empty statements and the pthread_cleanup* macros - * are pretty ill defined. */ -#if defined(__clang__) -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wextra-semi-stmt" -#endif - - Curl_thread_push_cleanup(async_thrd_cleanup, addr_ctx); - Curl_mutex_acquire(&addr_ctx->mutx); do_abort = addr_ctx->do_abort; Curl_mutex_release(&addr_ctx->mutx); if(!do_abort) { -#ifdef DEBUGBUILD - Curl_resolve_test_delay(); -#endif - addr_ctx->res = Curl_ipv4_resolve_r(addr_ctx->hostname, addr_ctx->port); if(!addr_ctx->res) { addr_ctx->sock_error = SOCKERRNO; @@ -337,12 +299,7 @@ static CURL_THREAD_RETURN_T CURL_STDCALL gethostbyname_thread(void *arg) #endif } - Curl_thread_pop_cleanup(); -#if defined(__clang__) -#pragma clang diagnostic pop -#endif - - async_thrd_cleanup(addr_ctx); + addr_ctx_unlink(&addr_ctx, NULL); return 0; } @@ -381,12 +338,12 @@ static void async_thrdd_destroy(struct Curl_easy *data) CURL_TRC_DNS(data, "async_thrdd_destroy, thread joined"); } else { - /* thread is still running. Detach the thread while mutexed, it will - * trigger the cleanup when it releases its reference. */ + /* thread is still running. Detach it. */ Curl_thread_destroy(&addr->thread_hnd); CURL_TRC_DNS(data, "async_thrdd_destroy, thread detached"); } } + /* release our reference to the shared context */ addr_ctx_unlink(&thrdd->addr, data); } @@ -532,10 +489,12 @@ static void async_thrdd_shutdown(struct Curl_easy *data) done = addr_ctx->thrd_done; Curl_mutex_release(&addr_ctx->mutx); - DEBUGASSERT(addr_ctx->thread_hnd != curl_thread_t_null); - if(!done && (addr_ctx->thread_hnd != curl_thread_t_null)) { - CURL_TRC_DNS(data, "cancelling resolve thread"); - (void)Curl_thread_cancel(&addr_ctx->thread_hnd); + /* Wait for the thread to terminate if it is already marked done. If it is + not done yet we cannot do anything here. We had tried pthread_cancel but + it caused hanging and resource leaks (#18532). */ + if(done && (addr_ctx->thread_hnd != curl_thread_t_null)) { + Curl_thread_join(&addr_ctx->thread_hnd); + CURL_TRC_DNS(data, "async_thrdd_shutdown, thread joined"); } } @@ -553,9 +512,11 @@ static CURLcode asyn_thrdd_await(struct Curl_easy *data, if(!entry) async_thrdd_shutdown(data); - CURL_TRC_DNS(data, "resolve, wait for thread to finish"); - if(!Curl_thread_join(&addr_ctx->thread_hnd)) { - DEBUGASSERT(0); + if(addr_ctx->thread_hnd != curl_thread_t_null) { + CURL_TRC_DNS(data, "resolve, wait for thread to finish"); + if(!Curl_thread_join(&addr_ctx->thread_hnd)) { + DEBUGASSERT(0); + } } if(entry) diff --git a/lib/curl_threads.c b/lib/curl_threads.c index 2750f5ad9f..96fd0f8a7a 100644 --- a/lib/curl_threads.c +++ b/lib/curl_threads.c @@ -100,34 +100,6 @@ int Curl_thread_join(curl_thread_t *hnd) return ret; } -/* do not use pthread_cancel if: - * - pthread_cancel seems to be absent - * - on FreeBSD, as we see hangers in CI testing - * - this is a -fsanitize=thread build - * (clang sanitizer reports false positive when functions to not return) - */ -#if defined(PTHREAD_CANCEL_ENABLE) && !defined(__FreeBSD__) -#if defined(__has_feature) -# if !__has_feature(thread_sanitizer) -#define USE_PTHREAD_CANCEL -# endif -#else /* __has_feature */ -#define USE_PTHREAD_CANCEL -#endif /* !__has_feature */ -#endif /* PTHREAD_CANCEL_ENABLE && !__FreeBSD__ */ - -int Curl_thread_cancel(curl_thread_t *hnd) -{ - (void)hnd; - if(*hnd != curl_thread_t_null) -#ifdef USE_PTHREAD_CANCEL - return pthread_cancel(**hnd); -#else - return 1; /* not supported */ -#endif - return 0; -} - #elif defined(USE_THREADS_WIN32) curl_thread_t Curl_thread_create(CURL_THREAD_RETURN_T @@ -182,12 +154,4 @@ int Curl_thread_join(curl_thread_t *hnd) return ret; } -int Curl_thread_cancel(curl_thread_t *hnd) -{ - if(*hnd != curl_thread_t_null) { - return 1; /* not supported */ - } - return 0; -} - #endif /* USE_THREADS_* */ diff --git a/lib/curl_threads.h b/lib/curl_threads.h index 115277c00e..82f08c5fbb 100644 --- a/lib/curl_threads.h +++ b/lib/curl_threads.h @@ -66,22 +66,6 @@ void Curl_thread_destroy(curl_thread_t *hnd); int Curl_thread_join(curl_thread_t *hnd); -int Curl_thread_cancel(curl_thread_t *hnd); - -#if defined(USE_THREADS_POSIX) && defined(PTHREAD_CANCEL_ENABLE) -#define Curl_thread_push_cleanup(a,b) pthread_cleanup_push(a,b) -#define Curl_thread_pop_cleanup() pthread_cleanup_pop(0) -#define Curl_thread_enable_cancel() \ - pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL) -#define Curl_thread_disable_cancel() \ - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL) -#else -#define Curl_thread_push_cleanup(a,b) ((void)a,(void)b) -#define Curl_thread_pop_cleanup() Curl_nop_stmt -#define Curl_thread_enable_cancel() Curl_nop_stmt -#define Curl_thread_disable_cancel() Curl_nop_stmt -#endif - #endif /* USE_THREADS_POSIX || USE_THREADS_WIN32 */ #endif /* HEADER_CURL_THREADS_H */ diff --git a/lib/hostip.c b/lib/hostip.c index 2d122fb999..b6be2ca1f2 100644 --- a/lib/hostip.c +++ b/lib/hostip.c @@ -1132,10 +1132,6 @@ CURLcode Curl_resolv_timeout(struct Curl_easy *data, prev_alarm = alarm(curlx_sltoui(timeout/1000L)); } -#ifdef DEBUGBUILD - Curl_resolve_test_delay(); -#endif - #else /* !USE_ALARM_TIMEOUT */ #ifndef CURLRES_ASYNCH if(timeoutms) @@ -1639,18 +1635,3 @@ CURLcode Curl_resolver_error(struct Curl_easy *data, const char *detail) return result; } #endif /* USE_CURL_ASYNC */ - -#ifdef DEBUGBUILD -#include "curlx/wait.h" - -void Curl_resolve_test_delay(void) -{ - const char *p = getenv("CURL_DNS_DELAY_MS"); - if(p) { - curl_off_t l; - if(!curlx_str_number(&p, &l, TIME_T_MAX) && l) { - curlx_wait_ms((timediff_t)l); - } - } -} -#endif diff --git a/lib/hostip.h b/lib/hostip.h index 2f78be82c2..3eb82cd149 100644 --- a/lib/hostip.h +++ b/lib/hostip.h @@ -216,8 +216,4 @@ struct Curl_addrinfo *Curl_sync_getaddrinfo(struct Curl_easy *data, #endif -#ifdef DEBUGBUILD -void Curl_resolve_test_delay(void); -#endif - #endif /* HEADER_CURL_HOSTIP_H */ diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index daed381e47..4523de4886 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -112,7 +112,7 @@ test754 test755 test756 test757 test758 test759 test760 test761 test762 \ test763 \ \ test780 test781 test782 test783 test784 test785 test786 test787 test788 \ -test789 test790 test791 test792 test793 test794 test795 test796 test797 \ +test789 test790 test791 test792 test793 test794 test796 test797 \ \ test799 test800 test801 test802 test803 test804 test805 test806 test807 \ test808 test809 test810 test811 test812 test813 test814 test815 test816 \ diff --git a/tests/data/test795 b/tests/data/test795 deleted file mode 100644 index 2c98b3bcc4..0000000000 --- a/tests/data/test795 +++ /dev/null @@ -1,36 +0,0 @@ - - - -DNS - - - -# Client-side - - -http -Debug -!c-ares -!win32 - - -Delayed resolve --connect-timeout check - - -CURL_DNS_DELAY_MS=5000 - - -http://test.invalid -v --no-progress-meter --trace-config dns --connect-timeout 1 -w \%{time_total} - - - -# Verify data after the test has been "shot" - - -28 - - -%SRCDIR/libtest/test795.pl %LOGDIR/stdout%TESTNUMBER 2 >> %LOGDIR/stderr%TESTNUMBER - - - diff --git a/tests/libtest/Makefile.am b/tests/libtest/Makefile.am index 4ccfbd2b7b..b62a359eab 100644 --- a/tests/libtest/Makefile.am +++ b/tests/libtest/Makefile.am @@ -42,7 +42,7 @@ AM_CPPFLAGS = -I$(top_srcdir)/include \ include Makefile.inc EXTRA_DIST = CMakeLists.txt $(FIRST_C) $(FIRST_H) $(UTILS_C) $(UTILS_H) $(TESTS_C) \ - test307.pl test610.pl test613.pl test795.pl test1013.pl test1022.pl mk-lib1521.pl + test307.pl test610.pl test613.pl test1013.pl test1022.pl mk-lib1521.pl CFLAGS += @CURL_CFLAG_EXTRAS@ diff --git a/tests/libtest/test795.pl b/tests/libtest/test795.pl deleted file mode 100755 index 6aa793f7f6..0000000000 --- a/tests/libtest/test795.pl +++ /dev/null @@ -1,46 +0,0 @@ -#!/usr/bin/env perl -#*************************************************************************** -# _ _ ____ _ -# Project ___| | | | _ \| | -# / __| | | | |_) | | -# | (__| |_| | _ <| |___ -# \___|\___/|_| \_\_____| -# -# Copyright (C) Daniel Stenberg, , et al. -# -# This software is licensed as described in the file COPYING, which -# you should have received as part of this distribution. The terms -# are also available at https://curl.se/docs/copyright.html. -# -# You may opt to use, copy, modify, merge, publish, distribute and/or sell -# copies of the Software, and permit persons to whom the Software is -# furnished to do so, under the terms of the COPYING file. -# -# This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY -# KIND, either express or implied. -# -# SPDX-License-Identifier: curl -# -########################################################################### -use strict; -use warnings; - -my $ok = 1; -my $exp_duration = $ARGV[1] + 0.0; - -# Read the output of curl --version -open(F, $ARGV[0]) || die "Can't open test result from $ARGV[0]\n"; -$_ = ; -chomp; -/\s*([\.\d]+)\s*/; -my $duration = $1 + 0.0; -close F; - -if ($duration <= $exp_duration) { - print "OK: duration of $duration in expected range\n"; - $ok = 0; -} -else { - print "FAILED: duration of $duration is larger than $exp_duration\n"; -} -exit $ok;