From 5bba43312c0ceccfe18bd4d086e12ec0497ed926 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 14 May 2024 17:04:37 +0100 Subject: build: Enable `thread_local` for MinGW-w64 builds The assumption in the commit 188ca75e5fe4837d16241446558c7566912f67b2 about the broken `thread_local` implementation in GCC was misguided because the initial failure was not due to GCC, but a bug in the Wine runtime, as evidenced, for example, here: - https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=917307 - https://bugs.freedesktop.org/show_bug.cgi?id=108662 Consequently, it is safe to re-enable `thread_local` support for MinGW-w64 builds. --- configure.ac | 6 ------ 1 file changed, 6 deletions(-) diff --git a/configure.ac b/configure.ac index bfc1e54921..b80466b13e 100644 --- a/configure.ac +++ b/configure.ac @@ -1043,12 +1043,6 @@ if test "$use_thread_local" = "yes" || test "$use_thread_local" = "auto"; then ])], [ case $host in - *mingw*) - dnl mingw32's implementation of thread_local has also been shown to behave - dnl erroneously under concurrent usage; see: - dnl https://gist.github.com/jamesob/fe9a872051a88b2025b1aa37bfa98605 - AC_MSG_RESULT([no]) - ;; *) AC_DEFINE([HAVE_THREAD_LOCAL], [1], [Define if thread_local is supported.]) AC_MSG_RESULT([yes]) -- cgit v1.2.3 From 1e7c20bc19a216269c646177ab90cfa084c096a5 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 21 May 2024 10:29:16 +0100 Subject: doc: remove comment about using thread_local Followup to https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1605655974. --- src/test/util/random.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/util/random.h b/src/test/util/random.h index c910bd6a3a..18ab425e48 100644 --- a/src/test/util/random.h +++ b/src/test/util/random.h @@ -14,9 +14,8 @@ /** * This global and the helpers that use it are not thread-safe. * - * If thread-safety is needed, the global could be made thread_local (given - * that thread_local is supported on all architectures we support) or a - * per-thread instance could be used in the multi-threaded test. + * If thread-safety is needed, a per-thread instance could be + * used in the multi-threaded test. */ extern FastRandomContext g_insecure_rand_ctx; -- cgit v1.2.3 From 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 18 May 2024 10:59:55 +0800 Subject: build: remove --enable-threadlocal --- configure.ac | 34 ---------------------------------- src/init/common.cpp | 8 +------- src/test/util_threadnames_tests.cpp | 7 ------- src/util/threadnames.cpp | 11 ----------- 4 files changed, 1 insertion(+), 59 deletions(-) diff --git a/configure.ac b/configure.ac index b80466b13e..d539e4436f 100644 --- a/configure.ac +++ b/configure.ac @@ -244,12 +244,6 @@ AC_ARG_ENABLE([lcov-branch-coverage], [use_lcov_branch=yes], [use_lcov_branch=no]) -AC_ARG_ENABLE([threadlocal], - [AS_HELP_STRING([--enable-threadlocal], - [enable features that depend on the c++ thread_local keyword (currently just thread names in debug logs). (default is to enable if there is platform support)])], - [use_thread_local=$enableval], - [use_thread_local=auto]) - AC_ARG_ENABLE([zmq], [AS_HELP_STRING([--disable-zmq], [disable ZMQ notifications])], @@ -1028,34 +1022,6 @@ AC_COMPILE_IFELSE([AC_LANG_SOURCE([ [AC_MSG_RESULT([no])] ) -if test "$use_thread_local" = "yes" || test "$use_thread_local" = "auto"; then - TEMP_LDFLAGS="$LDFLAGS" - LDFLAGS="$TEMP_LDFLAGS $PTHREAD_CFLAGS" - AC_MSG_CHECKING([for thread_local support]) - AC_LINK_IFELSE([AC_LANG_SOURCE([ - #include - static thread_local int foo = 0; - static void run_thread() { foo++;} - int main(){ - for(int i = 0; i < 10; i++) { std::thread(run_thread).detach();} - return foo; - } - ])], - [ - case $host in - *) - AC_DEFINE([HAVE_THREAD_LOCAL], [1], [Define if thread_local is supported.]) - AC_MSG_RESULT([yes]) - ;; - esac - ], - [ - AC_MSG_RESULT([no]) - ] - ) - LDFLAGS="$TEMP_LDFLAGS" -fi - dnl Check for different ways of gathering OS randomness AC_MSG_CHECKING([for Linux getrandom function]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ diff --git a/src/init/common.cpp b/src/init/common.cpp index 3a6df3e8bd..f473ee6d66 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -31,11 +31,7 @@ void AddLoggingArgs(ArgsManager& argsman) argsman.AddArg("-logips", strprintf("Include IP addresses in debug output (default: %u)", DEFAULT_LOGIPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-loglevel=|:", strprintf("Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC. Possible values are %s (default=%s). The following levels are always logged: error, warning, info. If : is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. can be: %s.", LogInstance().LogLevelsString(), LogInstance().LogLevelToStr(BCLog::DEFAULT_LOG_LEVEL), LogInstance().LogCategoriesString()), ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logtimestamps", strprintf("Prepend debug output with timestamp (default: %u)", DEFAULT_LOGTIMESTAMPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); -#ifdef HAVE_THREAD_LOCAL - argsman.AddArg("-logthreadnames", strprintf("Prepend debug output with name of the originating thread (only available on platforms supporting thread_local) (default: %u)", DEFAULT_LOGTHREADNAMES), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); -#else - argsman.AddHiddenArgs({"-logthreadnames"}); -#endif + argsman.AddArg("-logthreadnames", strprintf("Prepend debug output with name of the originating thread (default: %u)", DEFAULT_LOGTHREADNAMES), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logsourcelocations", strprintf("Prepend debug output with name of the originating source location (source file, line number and function name) (default: %u)", DEFAULT_LOGSOURCELOCATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-logtimemicros", strprintf("Add microsecond precision to debug timestamps (default: %u)", DEFAULT_LOGTIMEMICROS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-loglevelalways", strprintf("Always prepend a category and level (default: %u)", DEFAULT_LOGLEVELALWAYS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); @@ -50,9 +46,7 @@ void SetLoggingOptions(const ArgsManager& args) LogInstance().m_print_to_console = args.GetBoolArg("-printtoconsole", !args.GetBoolArg("-daemon", false)); LogInstance().m_log_timestamps = args.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS); LogInstance().m_log_time_micros = args.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS); -#ifdef HAVE_THREAD_LOCAL LogInstance().m_log_threadnames = args.GetBoolArg("-logthreadnames", DEFAULT_LOGTHREADNAMES); -#endif LogInstance().m_log_sourcelocations = args.GetBoolArg("-logsourcelocations", DEFAULT_LOGSOURCELOCATIONS); LogInstance().m_always_print_category_level = args.GetBoolArg("-loglevelalways", DEFAULT_LOGLEVELALWAYS); diff --git a/src/test/util_threadnames_tests.cpp b/src/test/util_threadnames_tests.cpp index df5b1a4461..174052d5fa 100644 --- a/src/test/util_threadnames_tests.cpp +++ b/src/test/util_threadnames_tests.cpp @@ -11,8 +11,6 @@ #include #include -#include // IWYU pragma: keep - #include BOOST_AUTO_TEST_SUITE(util_threadnames_tests) @@ -52,11 +50,6 @@ std::set RenameEnMasse(int num_threads) */ BOOST_AUTO_TEST_CASE(util_threadnames_test_rename_threaded) { -#if !defined(HAVE_THREAD_LOCAL) - // This test doesn't apply to platforms where we don't have thread_local. - return; -#endif - std::set names = RenameEnMasse(100); BOOST_CHECK_EQUAL(names.size(), 100U); diff --git a/src/util/threadnames.cpp b/src/util/threadnames.cpp index ba074e30e0..0249de37e3 100644 --- a/src/util/threadnames.cpp +++ b/src/util/threadnames.cpp @@ -37,10 +37,6 @@ static void SetThreadName(const char* name) #endif } -// If we have thread_local, just keep thread ID and name in a thread_local -// global. -#if defined(HAVE_THREAD_LOCAL) - /** * The name of the thread. We use char array instead of std::string to avoid * complications with running a destructor when the thread exits. Avoid adding @@ -58,13 +54,6 @@ static void SetInternalName(const std::string& name) g_thread_name[copy_bytes] = '\0'; } -// Without thread_local available, don't handle internal name at all. -#else - -std::string util::ThreadGetInternalName() { return ""; } -static void SetInternalName(const std::string& name) { } -#endif - void util::ThreadRename(const std::string& name) { SetThreadName(("b-" + name).c_str()); -- cgit v1.2.3