threads: introduce threadutil, refactor thread naming

jamesob · jamesob · commit 91cc480ddde3 · 2019-04-18T14:22:54.000-04:00
This work is prerequisite to attaching thread names to log lines and deadlock
debug utilities. threadutil allows setting of an "internal" threadname per
thread on platforms where thread_local is available.

This commit also moves RenameThread() out of util and adds a numeric suffix to
disambiguate between threads with the same name. It explicitly names a few
main threads using the new Rename() utility.
diff --git a/src/Makefile.am b/src/Makefile.am
@@ -194,6 +194,7 @@ BITCOIN_CORE_H = \
   support/events.h \
   support/lockedpool.h \
   sync.h \
+  threadutil.h \
   threadsafety.h \
   threadinterrupt.h \
   timedata.h \
@@ -483,6 +484,7 @@ libbitcoin_util_a_SOURCES = \
   rpc/protocol.cpp \
   support/cleanse.cpp \
   sync.cpp \
+  threadutil.cpp \
   threadinterrupt.cpp \
   util/bip32.cpp \
   util/bytevectorhash.cpp \
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
@@ -19,6 +19,7 @@
 #include <util/system.h>
 #include <httpserver.h>
 #include <httprpc.h>
+#include <threadutil.h>
 #include <util/strencodings.h>
 #include <walletinitinterface.h>
 
@@ -64,6 +65,8 @@ static bool AppInit(int argc, char* argv[])
 
     bool fRet = false;
 
+    thread_util::Rename("init");
+
     //
     // Parameters
     //
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
@@ -6,6 +6,7 @@
 
 #include <chainparamsbase.h>
 #include <compat.h>
+#include <threadutil.h>
 #include <util/system.h>
 #include <util/strencodings.h>
 #include <netbase.h>
@@ -17,7 +18,7 @@
 #include <memory>
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
+#include <string>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -284,7 +285,7 @@ static void http_reject_request_cb(struct evhttp_request* req, void*)
 /** Event dispatcher thread */
 static bool ThreadHTTP(struct event_base* base)
 {
-    RenameThread("bitcoin-http");
+    thread_util::Rename("http");
     LogPrint(BCLog::HTTP, "Entering http event loop\n");
     event_base_dispatch(base);
     // Event loop will be interrupted by InterruptHTTPServer()
@@ -335,9 +336,9 @@ static bool HTTPBindAddresses(struct evhttp* http)
 }
 
 /** Simple wrapper to set thread name and run work queue */
-static void HTTPWorkQueueRun(WorkQueue<HTTPClosure>* queue)
+static void HTTPWorkQueueRun(WorkQueue<HTTPClosure>* queue, int worker_num)
 {
-    RenameThread("bitcoin-httpworker");
+    thread_util::Rename(strprintf("httpworker.%i", worker_num));
     queue->Run();
 }
 
@@ -430,7 +431,7 @@ void StartHTTPServer()
     threadHTTP = std::thread(ThreadHTTP, eventBase);
 
     for (int i = 0; i < rpcThreads; i++) {
-        g_thread_http_workers.emplace_back(HTTPWorkQueueRun, workQueue);
+        g_thread_http_workers.emplace_back(HTTPWorkQueueRun, workQueue, i);
     }
 }
 
diff --git a/src/init.cpp b/src/init.cpp
@@ -42,6 +42,7 @@
 #include <script/sigcache.h>
 #include <scheduler.h>
 #include <shutdown.h>
+#include <threadutil.h>
 #include <timedata.h>
 #include <txdb.h>
 #include <txmempool.h>
@@ -208,7 +209,7 @@ void Shutdown(InitInterfaces& interfaces)
     /// for example if the data directory was found to be locked.
     /// Be sure that anything that writes files or flushes caches only does this if the respective
     /// module was initialized.
-    RenameThread("bitcoin-shutoff");
+    thread_util::Rename("shutoff");
     mempool.AddTransactionsUpdated(1);
 
     StopHTTPRPC();
@@ -669,7 +670,7 @@ static void CleanupBlockRevFiles()
 static void ThreadImport(std::vector<fs::path> vImportFiles)
 {
     const CChainParams& chainparams = Params();
-    RenameThread("bitcoin-loadblk");
+    thread_util::Rename("loadblk");
     ScheduleBatchPriority();
 
     {
@@ -1305,7 +1306,7 @@ bool AppInitMain(InitInterfaces& interfaces)
     LogPrintf("Using %u threads for script verification\n", nScriptCheckThreads);
     if (nScriptCheckThreads) {
         for (int i=0; i<nScriptCheckThreads-1; i++)
-            threadGroup.create_thread(&ThreadScriptCheck);
+            threadGroup.create_thread([i]() { return ThreadScriptCheck(i); });
     }
 
     // Start the lightweight task scheduler thread
diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
@@ -30,6 +30,7 @@
 #include <interfaces/handler.h>
 #include <interfaces/node.h>
 #include <noui.h>
+#include <threadutil.h>
 #include <rpc/server.h>
 #include <ui_interface.h>
 #include <uint256.h>
@@ -149,6 +150,7 @@ void BitcoinCore::initialize()
     try
     {
         qDebug() << __func__ << ": Running initialization in thread";
+        thread_util::Rename("qt-init");
         bool rv = m_node.appInitMain();
         Q_EMIT initializeResult(rv);
     } catch (const std::exception& e) {
@@ -423,6 +425,7 @@ int GuiMain(int argc, char* argv[])
     std::tie(argc, argv) = winArgs.get();
 #endif
     SetupEnvironment();
+    thread_util::Rename("main");
 
     std::unique_ptr<interfaces::Node> node = interfaces::MakeNode();
 
diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp
@@ -94,7 +94,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
 
     nScriptCheckThreads = 3;
     for (int i = 0; i < nScriptCheckThreads - 1; i++)
-        threadGroup.create_thread(&ThreadScriptCheck);
+        threadGroup.create_thread([i]() { return ThreadScriptCheck(i); });
 
     g_banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     g_connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests.
diff --git a/src/threadutil.cpp b/src/threadutil.cpp
@@ -0,0 +1,56 @@
+// Copyright (c) 2018 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#if defined(HAVE_CONFIG_H)
+#include <config/bitcoin-config.h>
+#endif
+
+#include <atomic>
+#include <thread>
+
+#include <threadutil.h>
+
+#ifdef HAVE_SYS_PRCTL_H
+#include <sys/prctl.h> // For prctl, PR_SET_NAME, PR_GET_NAME
+#endif
+
+//! Set the thread's name at the process level. Does not affect the
+//! internal name.
+static void SetThreadName(const char* name)
+{
+#if defined(PR_SET_NAME)
+    // Only the first 15 characters are used (16 - NUL terminator)
+    ::prctl(PR_SET_NAME, name, 0, 0, 0);
+#elif (defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__DragonFly__))
+    pthread_set_name_np(pthread_self(), name);
+#elif defined(MAC_OSX)
+    pthread_setname_np(name);
+#else
+    // Prevent warnings for unused parameters...
+    (void)name;
+#endif
+}
+
+// If we have thread_local, just keep thread ID and name in a thread_local
+// global.
+#if defined(HAVE_THREAD_LOCAL)
+
+static thread_local std::string g_thread_name;
+std::string thread_util::GetInternalName() { return g_thread_name; }
+//! Set the in-memory internal name for this thread. Does not affect the process
+//! name.
+static void SetInternalName(const std::string& name) { g_thread_name = name; }
+
+// Without thread_local available, don't handle internal name at all.
+#else
+
+std::string thread_util::GetInternalName() { return ""; }
+static void SetInternalName(const std::string& name) { }
+#endif
+
+void thread_util::Rename(const std::string& name)
+{
+    SetThreadName(("bitcoin-" + name).c_str());
+    SetInternalName(name);
+}
diff --git a/src/threadutil.h b/src/threadutil.h
@@ -0,0 +1,22 @@
+// Copyright (c) 2018 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_THREADUTIL_H
+#define BITCOIN_THREADUTIL_H
+
+#include <string>
+
+namespace thread_util
+{
+    //! Rename a thread both in terms of an internal (in-memory) name as well
+    //! as its system thread name.
+    void Rename(const std::string&);
+
+    //! Get the thread's internal (in-memory) name; used e.g. for identification in
+    //! logging.
+    std::string GetInternalName();
+
+} // namespace thread_util
+
+#endif // BITCOIN_THREADUTIL_H
diff --git a/src/util/system.cpp b/src/util/system.cpp
@@ -60,10 +60,6 @@
 #include <shlobj.h>
 #endif
 
-#ifdef HAVE_SYS_PRCTL_H
-#include <sys/prctl.h>
-#endif
-
 #ifdef HAVE_MALLOPT_ARENA_MAX
 #include <malloc.h>
 #endif
@@ -1137,22 +1133,6 @@ void runCommand(const std::string& strCommand)
         LogPrintf("runCommand error: system(%s) returned %d\n", strCommand, nErr);
 }
 
-void RenameThread(const char* name)
-{
-#if defined(PR_SET_NAME)
-    // Only the first 15 characters are used (16 - NUL terminator)
-    ::prctl(PR_SET_NAME, name, 0, 0, 0);
-#elif (defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__DragonFly__))
-    pthread_set_name_np(pthread_self(), name);
-
-#elif defined(MAC_OSX)
-    pthread_setname_np(name);
-#else
-    // Prevent warnings for unused parameters...
-    (void)name;
-#endif
-}
-
 void SetupEnvironment()
 {
 #ifdef HAVE_MALLOPT_ARENA_MAX
diff --git a/src/util/system.h b/src/util/system.h
@@ -20,6 +20,7 @@
 #include <fs.h>
 #include <logging.h>
 #include <sync.h>
+#include <threadutil.h>
 #include <tinyformat.h>
 #include <util/memory.h>
 #include <util/time.h>
@@ -325,15 +326,12 @@ std::string HelpMessageOpt(const std::string& option, const std::string& message
  */
 int GetNumCores();
 
-void RenameThread(const char* name);
-
 /**
  * .. and a wrapper that just calls func once
  */
 template <typename Callable> void TraceThread(const char* name,  Callable func)
 {
-    std::string s = strprintf("bitcoin-%s", name);
-    RenameThread(s.c_str());
+    thread_util::Rename(name);
     try
     {
         LogPrintf("%s thread start\n", name);
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -48,6 +48,7 @@
 
 #include <future>
 #include <sstream>
+#include <string>
 
 #include <boost/algorithm/string/replace.hpp>
 #include <boost/thread.hpp>
@@ -1654,8 +1655,8 @@ static bool WriteUndoDataForBlock(const CBlockUndo& blockundo, CValidationState&
 
 static CCheckQueue<CScriptCheck> scriptcheckqueue(128);
 
-void ThreadScriptCheck() {
-    RenameThread("bitcoin-scriptch");
+void ThreadScriptCheck(int worker_num) {
+    thread_util::Rename(strprintf("scriptch.%i", worker_num));
     scriptcheckqueue.Thread();
 }
 
diff --git a/src/validation.h b/src/validation.h
@@ -255,7 +255,7 @@ bool LoadChainTip(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_m
 /** Unload database information */
 void UnloadBlockIndex();
 /** Run an instance of the script checking thread */
-void ThreadScriptCheck();
+void ThreadScriptCheck(int worker_num);
 /** Check whether we are doing an initial block download (synchronizing from disk or network) */
 bool IsInitialBlockDownload();
 /** Retrieve a transaction (from memory pool, or from disk, if possible) */
