diff options
author | merge-script <fanquake@gmail.com> | 2024-05-21 10:25:56 +0100 |
---|---|---|
committer | merge-script <fanquake@gmail.com> | 2024-05-21 10:25:56 +0100 |
commit | 8804ec736adaacf5392392add003e679069ad527 (patch) | |
tree | 52116c6bcf0a85fa2a6a7357c3933844ab8cfad6 /src | |
parent | 5acdc2b97dcd756a15d28d75bce9a9d3e3953f9f (diff) | |
parent | d35ba1b3f16071b8fe9b36398ba15352dbf2a54d (diff) | |
download | bitcoin-8804ec736adaacf5392392add003e679069ad527.tar.xz |
Merge bitcoin/bitcoin#30095: util: avoid using thread_local variable that has a destructor
d35ba1b3f16071b8fe9b36398ba15352dbf2a54d util: avoid using thread_local variable that has a destructor (Vasil Dimov)
Pull request description:
Store the thread name in a `thread_local` variable of type `char[]` instead of `std::string`. This avoids calling the destructor when the thread exits. This is a workaround for
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278701
For type-safety, return `std::string` from
`util::ThreadGetInternalName()` instead of `char[]`.
As a side effect of this change, we no longer store a reference to a `thread_local` variable in `CLockLocation`. This was dangerous because if the thread quits while the reference still exists (in the global variable `lock_data`, see inside `GetLockData()`) then the reference will become dangling.
ACKs for top commit:
laanwj:
Code review ACK d35ba1b3f16071b8fe9b36398ba15352dbf2a54d
hebasto:
re-ACK d35ba1b3f16071b8fe9b36398ba15352dbf2a54d.
theuni:
utACK d35ba1b3f16071b8fe9b36398ba15352dbf2a54d
Tree-SHA512: a2a3bc4401654d6e99db5b9c46a7051855f5a26886142298662e681b78dd581ff4c6bebe42f649b8e1fb8a78d569c6117302db2cd6362e884a22f2a5839b7d43
Diffstat (limited to 'src')
-rw-r--r-- | src/sync.cpp | 6 | ||||
-rw-r--r-- | src/util/threadnames.cpp | 31 | ||||
-rw-r--r-- | src/util/threadnames.h | 6 |
3 files changed, 27 insertions, 16 deletions
diff --git a/src/sync.cpp b/src/sync.cpp index a8bdfc1dea..93c9194541 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -37,11 +37,11 @@ struct CLockLocation { const char* pszFile, int nLine, bool fTryIn, - const std::string& thread_name) + std::string&& thread_name) : fTry(fTryIn), mutexName(pszName), sourceFile(pszFile), - m_thread_name(thread_name), + m_thread_name(std::move(thread_name)), sourceLine(nLine) {} std::string ToString() const @@ -60,7 +60,7 @@ private: bool fTry; std::string mutexName; std::string sourceFile; - const std::string& m_thread_name; + const std::string m_thread_name; int sourceLine; }; diff --git a/src/util/threadnames.cpp b/src/util/threadnames.cpp index ea597dd05a..ba074e30e0 100644 --- a/src/util/threadnames.cpp +++ b/src/util/threadnames.cpp @@ -4,6 +4,7 @@ #include <config/bitcoin-config.h> // IWYU pragma: keep +#include <cstring> #include <string> #include <thread> #include <utility> @@ -40,27 +41,37 @@ static void SetThreadName(const char* name) // global. #if defined(HAVE_THREAD_LOCAL) -static thread_local std::string g_thread_name; -const std::string& util::ThreadGetInternalName() { return g_thread_name; } +/** + * 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 + * other thread_local variables. + * @see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278701 + */ +static thread_local char g_thread_name[128]{'\0'}; +std::string util::ThreadGetInternalName() { return g_thread_name; } //! Set the in-memory internal name for this thread. Does not affect the process //! name. -static void SetInternalName(std::string name) { g_thread_name = std::move(name); } +static void SetInternalName(const std::string& name) +{ + const size_t copy_bytes{std::min(sizeof(g_thread_name) - 1, name.length())}; + std::memcpy(g_thread_name, name.data(), copy_bytes); + g_thread_name[copy_bytes] = '\0'; +} // Without thread_local available, don't handle internal name at all. #else -static const std::string empty_string; -const std::string& util::ThreadGetInternalName() { return empty_string; } -static void SetInternalName(std::string name) { } +std::string util::ThreadGetInternalName() { return ""; } +static void SetInternalName(const std::string& name) { } #endif -void util::ThreadRename(std::string&& name) +void util::ThreadRename(const std::string& name) { SetThreadName(("b-" + name).c_str()); - SetInternalName(std::move(name)); + SetInternalName(name); } -void util::ThreadSetInternalName(std::string&& name) +void util::ThreadSetInternalName(const std::string& name) { - SetInternalName(std::move(name)); + SetInternalName(name); } diff --git a/src/util/threadnames.h b/src/util/threadnames.h index 64b2689cf1..adca8c3000 100644 --- a/src/util/threadnames.h +++ b/src/util/threadnames.h @@ -12,14 +12,14 @@ namespace util { //! as its system thread name. //! @note Do not call this for the main thread, as this will interfere with //! UNIX utilities such as top and killall. Use ThreadSetInternalName instead. -void ThreadRename(std::string&&); +void ThreadRename(const std::string&); //! Set the internal (in-memory) name of the current thread only. -void ThreadSetInternalName(std::string&&); +void ThreadSetInternalName(const std::string&); //! Get the thread's internal (in-memory) name; used e.g. for identification in //! logging. -const std::string& ThreadGetInternalName(); +std::string ThreadGetInternalName(); } // namespace util |