← Commit list

de3fc1d7ad

Stefan Eissing
Stefan Eissing
2025-09-15
FAILED Duration: 200s

Subject

asyn-thrdd: drop pthread_cancel

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

docs/libcurl/libcurl-env-dbg.md +0 -5lib/asyn-thrdd.c +14 -53lib/curl_threads.c +0 -36lib/curl_threads.h +0 -16lib/hostip.c +0 -19lib/hostip.h +0 -4tests/data/Makefile.am +1 -1tests/data/test795 +0 -36tests/libtest/Makefile.am +1 -1tests/libtest/test795.pl +0 -46
docs/libcurl/libcurl-env-dbg.md+0 -5
@@ -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
lib/asyn-thrdd.c+14 -53
@@ -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)
lib/curl_threads.c+0 -36
@@ -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_* */
lib/curl_threads.h+0 -16
@@ -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 */
lib/hostip.c+0 -19
@@ -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
lib/hostip.h+0 -4
@@ -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 */
tests/data/Makefile.am+1 -1
@@ -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 \
tests/data/test795+0 -36
@@ -1,36 +0,0 @@
-<testcase>
-<info>
-<keywords>
-DNS
-</keywords>
-</info>
-
-# Client-side
-<client>
-<features>
-http
-Debug
-!c-ares
-!win32
-</features>
-<name>
-Delayed resolve --connect-timeout check
-</name>
-<setenv>
-CURL_DNS_DELAY_MS=5000
-</setenv>
-<command>
-http://test.invalid -v --no-progress-meter --trace-config dns --connect-timeout 1 -w \%{time_total}
-</command>
-</client>
-
-# Verify data after the test has been "shot"
-<verify>
-<errorcode>
-28
-</errorcode>
-<postcheck>
-%SRCDIR/libtest/test795.pl %LOGDIR/stdout%TESTNUMBER 2 >> %LOGDIR/stderr%TESTNUMBER
-</postcheck>
-</verify>
-</testcase>
tests/libtest/Makefile.am+1 -1
@@ -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@
tests/libtest/test795.pl+0 -46
@@ -1,46 +0,0 @@
-#!/usr/bin/env perl
-#***************************************************************************
-# _ _ ____ _
-# Project ___| | | | _ \| |
-# / __| | | | |_) | |
-# | (__| |_| | _ <| |___
-# \___|\___/|_| \_\_____|
-#
-# Copyright (C) Daniel Stenberg, <[email protected]>, 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";
-$_ = <F>;
-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;