Merge bitcoin/bitcoin#19064: refactor: Cleanup thread ctor calls

792be53d3e9e366b9f6aeee7a1eeb912fa28062e refactor: Replace std::bind with lambdas (Hennadii Stepanov)
a508f718f3e087c96a306399582a85df2e1d53ae refactor: Use appropriate thread constructor (Hennadii Stepanov)
30e44482152488a78f2c495798a75e6f553dc0c8 refactor: Make TraceThread a non-template free function (Hennadii Stepanov)

Pull request description:

  This PR does not change behavior.
  Its goal is to improve readability and maintainability of the code.

ACKs for top commit:
  jnewbery:
    utACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
  jonatack:
    tACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
  MarcoFalke:
    cr ACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e

Tree-SHA512: a03142f04f370f6bc02bd3ddfa870819b51740fcd028772241d68c84087f95a2d78207cbd5edb3f7c636fcf2d76192d9c59873f8f0af451d3b05c0cf9cf234df

11 files changed, 71 insertions, 39 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 972a3e279b..41fab024c6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -257,6 +257,7 @@ BITCOIN_CORE_H = \
   util/spanparsing.h \
   util/string.h \
   util/system.h \
+  util/thread.h \
   util/threadnames.h \
   util/time.h \
   util/tokenpipe.h \
@@ -590,6 +591,7 @@ libbitcoin_util_a_SOURCES = \
   util/rbf.cpp \
   util/readwritefile.cpp \
   util/settings.cpp \
+  util/thread.cpp \
   util/threadnames.cpp \
   util/spanparsing.cpp \
   util/strencodings.cpp \
diff --git a/src/index/base.cpp b/src/index/base.cpp
index 357c4fbaf9..0ab49f8a37 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -8,7 +8,7 @@
 #include <node/ui_interface.h>
 #include <shutdown.h>
 #include <tinyformat.h>
-#include <util/system.h>
+#include <util/thread.h>
 #include <util/translation.h>
 #include <validation.h> // For g_chainman
 #include <warnings.h>
@@ -349,8 +349,7 @@ void BaseIndex::Start()
         return;
     }
 
-    m_thread_sync = std::thread(&TraceThread<std::function<void()>>, GetName(),
-                                std::bind(&BaseIndex::ThreadSync, this));
+    m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { ThreadSync(); });
 }
 
 void BaseIndex::Stop()
diff --git a/src/init.cpp b/src/init.cpp
index a0241f4c3d..481c3c7d63 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -59,6 +59,7 @@
 #include <util/moneystr.h>
 #include <util/string.h>
 #include <util/system.h>
+#include <util/thread.h>
 #include <util/threadnames.h>
 #include <util/translation.h>
 #include <validation.h>
@@ -1114,7 +1115,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     node.scheduler = std::make_unique<CScheduler>();
 
     // Start the lightweight task scheduler thread
-    node.scheduler->m_service_thread = std::thread([&] { TraceThread("scheduler", [&] { node.scheduler->serviceQueue(); }); });
+    node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { node.scheduler->serviceQueue(); });
 
     // Gather some entropy once per minute.
     node.scheduler->scheduleEvery([]{
@@ -1629,7 +1630,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         vImportFiles.push_back(strFile);
     }
 
-    chainman.m_load_block = std::thread(&TraceThread<std::function<void()>>, "loadblk", [=, &chainman, &args] {
+    chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &chainman, &args] {
         ThreadImport(chainman, vImportFiles, args);
     });
 
diff --git a/src/mapport.cpp b/src/mapport.cpp
index 2df4ce45d2..135efb561e 100644
--- a/src/mapport.cpp
+++ b/src/mapport.cpp
@@ -15,6 +15,7 @@
 #include <netbase.h>
 #include <threadinterrupt.h>
 #include <util/system.h>
+#include <util/thread.h>
 
 #ifdef USE_NATPMP
 #include <compat.h>
@@ -255,7 +256,7 @@ void StartThreadMapPort()
 {
     if (!g_mapport_thread.joinable()) {
         assert(!g_mapport_interrupt);
-        g_mapport_thread = std::thread(std::bind(&TraceThread<void (*)()>, "mapport", &ThreadMapPort));
+        g_mapport_thread = std::thread(&util::TraceThread, "mapport", &ThreadMapPort);
     }
 }
 
diff --git a/src/net.cpp b/src/net.cpp
index ea5d11da18..bbc0b81056 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -23,6 +23,7 @@
 #include <scheduler.h>
 #include <util/sock.h>
 #include <util/strencodings.h>
+#include <util/thread.h>
 #include <util/translation.h>
 
 #ifdef WIN32
@@ -2519,15 +2520,15 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
     }
 
     // Send and receive from sockets, accept connections
-    threadSocketHandler = std::thread(&TraceThread<std::function<void()> >, "net", std::function<void()>(std::bind(&CConnman::ThreadSocketHandler, this)));
+    threadSocketHandler = std::thread(&util::TraceThread, "net", [this] { ThreadSocketHandler(); });
 
     if (!gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED))
         LogPrintf("DNS seeding disabled\n");
     else
-        threadDNSAddressSeed = std::thread(&TraceThread<std::function<void()> >, "dnsseed", std::function<void()>(std::bind(&CConnman::ThreadDNSAddressSeed, this)));
+        threadDNSAddressSeed = std::thread(&util::TraceThread, "dnsseed", [this] { ThreadDNSAddressSeed(); });
 
     // Initiate manual connections
-    threadOpenAddedConnections = std::thread(&TraceThread<std::function<void()> >, "addcon", std::function<void()>(std::bind(&CConnman::ThreadOpenAddedConnections, this)));
+    threadOpenAddedConnections = std::thread(&util::TraceThread, "addcon", [this] { ThreadOpenAddedConnections(); });
 
     if (connOptions.m_use_addrman_outgoing && !connOptions.m_specified_outgoing.empty()) {
         if (clientInterface) {
@@ -2537,16 +2538,18 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
         }
         return false;
     }
-    if (connOptions.m_use_addrman_outgoing || !connOptions.m_specified_outgoing.empty())
-        threadOpenConnections = std::thread(&TraceThread<std::function<void()> >, "opencon", std::function<void()>(std::bind(&CConnman::ThreadOpenConnections, this, connOptions.m_specified_outgoing)));
+    if (connOptions.m_use_addrman_outgoing || !connOptions.m_specified_outgoing.empty()) {
+        threadOpenConnections = std::thread(
+            &util::TraceThread, "opencon",
+            [this, connect = connOptions.m_specified_outgoing] { ThreadOpenConnections(connect); });
+    }
 
     // Process messages
-    threadMessageHandler = std::thread(&TraceThread<std::function<void()> >, "msghand", std::function<void()>(std::bind(&CConnman::ThreadMessageHandler, this)));
+    threadMessageHandler = std::thread(&util::TraceThread, "msghand", [this] { ThreadMessageHandler(); });
 
     if (connOptions.m_i2p_accept_incoming && m_i2p_sam_session.get() != nullptr) {
         threadI2PAcceptIncoming =
-            std::thread(&TraceThread<std::function<void()>>, "i2paccept",
-                        std::function<void()>(std::bind(&CConnman::ThreadI2PAcceptIncoming, this)));
+            std::thread(&util::TraceThread, "i2paccept", [this] { ThreadI2PAcceptIncoming(); });
     }
 
     // Dump network addresses
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index ffc5115145..f92e4c4b99 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -28,6 +28,8 @@
 #include <txdb.h>
 #include <util/strencodings.h>
 #include <util/string.h>
+#include <util/thread.h>
+#include <util/threadnames.h>
 #include <util/time.h>
 #include <util/translation.h>
 #include <util/url.h>
@@ -135,7 +137,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
     // We have to run a scheduler thread to prevent ActivateBestChain
     // from blocking due to queue overrun.
     m_node.scheduler = std::make_unique<CScheduler>();
-    m_node.scheduler->m_service_thread = std::thread([&] { TraceThread("scheduler", [&] { m_node.scheduler->serviceQueue(); }); });
+    m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); });
     GetMainSignals().RegisterBackgroundSignalScheduler(*m_node.scheduler);
 
     pblocktree.reset(new CBlockTreeDB(1 << 20, true));
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index 6666e49a2b..19d0a5da81 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -15,6 +15,7 @@
 #include <util/readwritefile.h>
 #include <util/strencodings.h>
 #include <util/system.h>
+#include <util/thread.h>
 #include <util/time.h>
 
 #include <deque>
@@ -596,7 +597,7 @@ void StartTorControl(CService onion_service_target)
         return;
     }
 
-    torControlThread = std::thread(&TraceThread<std::function<void()>>, "torcontrol", [onion_service_target] {
+    torControlThread = std::thread(&util::TraceThread, "torcontrol", [onion_service_target] {
         TorControlThread(onion_service_target);
     });
 }
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 67549fc13d..5957637e81 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -18,6 +18,7 @@
 #include <validation.h>
 #include <validationinterface.h>
 
+#include <cmath>
 #include <optional>
 
 CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee,
diff --git a/src/util/system.h b/src/util/system.h
index 61f862c93a..f68975ffa3 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -22,7 +22,6 @@
 #include <sync.h>
 #include <tinyformat.h>
 #include <util/settings.h>
-#include <util/threadnames.h>
 #include <util/time.h>
 
 #include <any>
@@ -478,28 +477,6 @@ std::string HelpMessageOpt(const std::string& option, const std::string& message
  */
 int GetNumCores();
 
-/**
- * .. and a wrapper that just calls func once
- */
-template <typename Callable> void TraceThread(const char* name,  Callable func)
-{
-    util::ThreadRename(name);
-    try
-    {
-        LogPrintf("%s thread start\n", name);
-        func();
-        LogPrintf("%s thread exit\n", name);
-    }
-    catch (const std::exception& e) {
-        PrintExceptionContinue(&e, name);
-        throw;
-    }
-    catch (...) {
-        PrintExceptionContinue(nullptr, name);
-        throw;
-    }
-}
-
 std::string CopyrightHolders(const std::string& strPrefix);
 
 /**
diff --git a/src/util/thread.cpp b/src/util/thread.cpp
new file mode 100644
index 0000000000..14be668685
--- /dev/null
+++ b/src/util/thread.cpp
@@ -0,0 +1,27 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <util/thread.h>
+
+#include <logging.h>
+#include <util/system.h>
+#include <util/threadnames.h>
+
+#include <exception>
+
+void util::TraceThread(const char* thread_name, std::function<void()> thread_func)
+{
+    util::ThreadRename(thread_name);
+    try {
+        LogPrintf("%s thread start\n", thread_name);
+        thread_func();
+        LogPrintf("%s thread exit\n", thread_name);
+    } catch (const std::exception& e) {
+        PrintExceptionContinue(&e, thread_name);
+        throw;
+    } catch (...) {
+        PrintExceptionContinue(nullptr, thread_name);
+        throw;
+    }
+}
diff --git a/src/util/thread.h b/src/util/thread.h
new file mode 100644
index 0000000000..ca2eccc0c3
--- /dev/null
+++ b/src/util/thread.h
@@ -0,0 +1,18 @@
+// Copyright (c) 2021 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_UTIL_THREAD_H
+#define BITCOIN_UTIL_THREAD_H
+
+#include <functional>
+
+namespace util {
+/**
+ * A wrapper for do-something-once thread functions.
+ */
+void TraceThread(const char* thread_name, std::function<void()> thread_func);
+
+} // namespace util
+
+#endif // BITCOIN_UTIL_THREAD_H