From 188ca75e5fe4837d16241446558c7566912f67b2 Mon Sep 17 00:00:00 2001
From: James O'Beirne <james.obeirne@gmail.com>
Date: Mon, 21 May 2018 13:44:23 -0400
Subject: [PATCH 1/5] disable HAVE_THREAD_LOCAL on unreliable platforms
Note that this doesn't affect anything unless
DEBUG_LOCKCONTENTION is defined.
See discussions here:
- https://github.com/bitcoin/bitcoin/pull/11722#pullrequestreview-79322658
- https://github.com/bitcoin/bitcoin/pull/13168#issuecomment-387181155
---
configure.ac | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index a3ba8ce80..302fc639d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -827,8 +827,23 @@ AC_LINK_IFELSE([AC_LANG_SOURCE([
}
])],
[
- AC_DEFINE(HAVE_THREAD_LOCAL,1,[Define if thread_local is supported.])
- AC_MSG_RESULT(yes)
+ case $host in
+ *mingw*)
+ # mingw32's implementation of thread_local has also been shown to behave
+ # erroneously under concurrent usage; see:
+ # https://gist.github.com/jamesob/fe9a872051a88b2025b1aa37bfa98605
+ AC_MSG_RESULT(no)
+ ;;
+ *darwin*)
+ # TODO enable thread_local on later versions of Darwin where it is
+ # supported (per https://stackoverflow.com/a/29929949)
+ AC_MSG_RESULT(no)
+ ;;
+ *)
+ AC_DEFINE(HAVE_THREAD_LOCAL,1,[Define if thread_local is supported.])
+ AC_MSG_RESULT(yes)
+ ;;
+ esac
],
[
AC_MSG_RESULT(no)
From ae5f2b6a6cc7b2260e9dff99c1bf378922e0e988 Mon Sep 17 00:00:00 2001
From: James O'Beirne <james.obeirne@gmail.com>
Date: Wed, 13 Jun 2018 14:50:59 -0400
Subject: [PATCH 2/5] threads: introduce util/threadnames, refactor thread
naming
This work is prerequisite to attaching thread names to log lines and deadlock
debug utilities. This code allows setting of an "internal" threadname per
thread on platforms where thread_local is available.
This commit also moves RenameThread() out of a more general module and adds a
numeric suffix to disambiguate between threads with the same name. It
explicitly names a few main threads using the new util::ThreadRename().
---
src/Makefile.am | 2 ++
src/bitcoind.cpp | 3 +++
src/httpserver.cpp | 11 ++++----
src/init.cpp | 7 ++---
src/qt/bitcoin.cpp | 3 +++
src/test/setup_common.cpp | 2 +-
src/util/system.cpp | 20 --------------
src/util/system.h | 6 ++---
src/util/threadnames.cpp | 57 +++++++++++++++++++++++++++++++++++++++
src/util/threadnames.h | 21 +++++++++++++++
src/validation.cpp | 5 ++--
src/validation.h | 2 +-
12 files changed, 103 insertions(+), 36 deletions(-)
create mode 100644 src/util/threadnames.cpp
create mode 100644 src/util/threadnames.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 059ed1813..0fefa34cd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -210,6 +210,7 @@ BITCOIN_CORE_H = \
util/memory.h \
util/moneystr.h \
util/rbf.h \
+ util/threadnames.h \
util/time.h \
util/url.h \
util/validation.h \
@@ -491,6 +492,7 @@ libbitcoin_util_a_SOURCES = \
util/system.cpp \
util/moneystr.cpp \
util/rbf.cpp \
+ util/threadnames.cpp \
util/strencodings.cpp \
util/time.cpp \
util/url.cpp \
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
index dde75c1b1..b31f86cdd 100644
--- a/src/bitcoind.cpp
+++ b/src/bitcoind.cpp
@@ -19,6 +19,7 @@
#include <util/system.h>
#include <httpserver.h>
#include <httprpc.h>
+#include <util/threadnames.h>
#include <util/strencodings.h>
#include <walletinitinterface.h>
@@ -64,6 +65,8 @@ static bool AppInit(int argc, char* argv[])
bool fRet = false;
+ util::ThreadRename("init");
+
//
// Parameters
//
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 5d9c3d2c1..63639fa3e 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -6,6 +6,7 @@
#include <chainparamsbase.h>
#include <compat.h>
+#include <util/threadnames.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");
+ util::ThreadRename("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");
+ util::ThreadRename(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
index 70459994c..408a133e4 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -42,6 +42,7 @@
#include <script/sigcache.h>
#include <scheduler.h>
#include <shutdown.h>
+#include <util/threadnames.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");
+ util::ThreadRename("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");
+ util::ThreadRename("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
index 1b063771e..81255aaae 100644
--- 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 <util/threadnames.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";
+ util::ThreadRename("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();
+ util::ThreadRename("main");
std::unique_ptr<interfaces::Node> node = interfaces::MakeNode();
diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp
index 29633cc7b..6b671b240 100644
--- 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/util/system.cpp b/src/util/system.cpp
index 9594dd81b..efd35bed5 100644
--- 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
index 54eb88e26..1a83cb67b 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -20,6 +20,7 @@
#include <fs.h>
#include <logging.h>
#include <sync.h>
+#include <util/threadnames.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());
+ util::ThreadRename(name);
try
{
LogPrintf("%s thread start\n", name);
diff --git a/src/util/threadnames.cpp b/src/util/threadnames.cpp
new file mode 100644
index 000000000..7b0d744ae
--- /dev/null
+++ b/src/util/threadnames.cpp
@@ -0,0 +1,57 @@
+// 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 <util/threadnames.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;
+const 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); }
+
+// 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) { }
+#endif
+
+void util::ThreadRename(std::string&& name)
+{
+ SetThreadName(("bitcoin-" + name).c_str());
+ SetInternalName(std::move(name));
+}
diff --git a/src/util/threadnames.h b/src/util/threadnames.h
new file mode 100644
index 000000000..aaf07b9bf
--- /dev/null
+++ b/src/util/threadnames.h
@@ -0,0 +1,21 @@
+// 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_UTIL_THREADNAMES_H
+#define BITCOIN_UTIL_THREADNAMES_H
+
+#include <string>
+
+namespace util {
+//! Rename a thread both in terms of an internal (in-memory) name as well
+//! as its system thread name.
+void ThreadRename(std::string&&);
+
+//! Get the thread's internal (in-memory) name; used e.g. for identification in
+//! logging.
+const std::string& ThreadGetInternalName();
+
+} // namespace util
+
+#endif // BITCOIN_UTIL_THREADNAMES_H
diff --git a/src/validation.cpp b/src/validation.cpp
index be6257ea2..208cf2c3f 100644
--- 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) {
+ util::ThreadRename(strprintf("scriptch.%i", worker_num));
scriptcheckqueue.Thread();
}
diff --git a/src/validation.h b/src/validation.h
index 041d8860b..82151b134 100644
--- 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) */
From ddd95ccb80da3d5cdf905192e23d54f303e6d83f Mon Sep 17 00:00:00 2001
From: James O'Beirne <james.obeirne@gmail.com>
Date: Sat, 5 May 2018 14:11:46 -0400
Subject: [PATCH 3/5] tests: add threadutil tests
---
src/Makefile.test.include | 1 +
src/test/util_threadnames_tests.cpp | 73 +++++++++++++++++++++++++++++
2 files changed, 74 insertions(+)
create mode 100644 src/test/util_threadnames_tests.cpp
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 692f8d97b..1144ca8a7 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -138,6 +138,7 @@ BITCOIN_TESTS =\
test/skiplist_tests.cpp \
test/streams_tests.cpp \
test/sync_tests.cpp \
+ test/util_threadnames_tests.cpp \
test/timedata_tests.cpp \
test/torcontrol_tests.cpp \
test/transaction_tests.cpp \
diff --git a/src/test/util_threadnames_tests.cpp b/src/test/util_threadnames_tests.cpp
new file mode 100644
index 000000000..71c0168ca
--- /dev/null
+++ b/src/test/util_threadnames_tests.cpp
@@ -0,0 +1,73 @@
+// 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.
+
+#include <util/threadnames.h>
+#include <test/setup_common.h>
+
+#include <thread>
+#include <vector>
+#include <set>
+#include <mutex>
+
+#if defined(HAVE_CONFIG_H)
+#include <config/bitcoin-config.h>
+#endif
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(util_threadnames_tests, BasicTestingSetup)
+
+const std::string TEST_THREAD_NAME_BASE = "test_thread.";
+
+/**
+ * Run a bunch of threads to all call util::ThreadRename.
+ *
+ * @return the set of name each thread has after attempted renaming.
+ */
+std::set<std::string> RenameEnMasse(int num_threads)
+{
+ std::vector<std::thread> threads;
+ std::set<std::string> names;
+ std::mutex lock;
+
+ auto RenameThisThread = [&](int i) {
+ util::ThreadRename(TEST_THREAD_NAME_BASE + std::to_string(i));
+ std::lock_guard<std::mutex> guard(lock);
+ names.insert(util::ThreadGetInternalName());
+ };
+
+ for (int i = 0; i < num_threads; ++i) {
+ threads.push_back(std::thread(RenameThisThread, i));
+ }
+
+ for (std::thread& thread : threads) thread.join();
+
+ return names;
+}
+
+/**
+ * Rename a bunch of threads with the same basename (expect_multiple=true), ensuring suffixes are
+ * applied properly.
+ */
+BOOST_AUTO_TEST_CASE(util_threadnames_test_rename_threaded)
+{
+ BOOST_CHECK_EQUAL(util::ThreadGetInternalName(), "");
+
+#if !defined(HAVE_THREAD_LOCAL)
+ // This test doesn't apply to platforms where we don't have thread_local.
+ return;
+#endif
+
+ std::set<std::string> names = RenameEnMasse(100);
+
+ BOOST_CHECK_EQUAL(names.size(), 100);
+
+ // Names "test_thread.[n]" should exist for n = [0, 99]
+ for (int i = 0; i < 100; ++i) {
+ BOOST_CHECK(names.find(TEST_THREAD_NAME_BASE + std::to_string(i)) != names.end());
+ }
+
+}
+
+BOOST_AUTO_TEST_SUITE_END()
From 383b186c28979e42adbdd257a40114c0e3f14a83 Mon Sep 17 00:00:00 2001
From: James O'Beirne <james@chaincode.com>
Date: Sat, 5 May 2018 14:14:43 -0400
Subject: [PATCH 4/5] threads: prefix log messages with thread names
Introduce a new flag (`-logthreadnames`) which allows toggling
of this behavior.
---
doc/release-notes-15849.md | 6 ++++++
src/init.cpp | 2 ++
src/logging.cpp | 26 ++++++++++++++++----------
src/logging.h | 2 ++
4 files changed, 26 insertions(+), 10 deletions(-)
create mode 100644 doc/release-notes-15849.md
diff --git a/doc/release-notes-15849.md b/doc/release-notes-15849.md
new file mode 100644
index 000000000..a1df31f25
--- /dev/null
+++ b/doc/release-notes-15849.md
@@ -0,0 +1,6 @@
+Thread names in logs
+--------------------
+
+On platforms supporting `thread_local`, log lines can be prefixed with the name
+of the thread that caused the log. To enable this behavior, use
+`-logthreadnames=1`.
diff --git a/src/init.cpp b/src/init.cpp
index 408a133e4..84fb313e9 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -508,6 +508,7 @@ void SetupServerArgs()
gArgs.AddArg("-debugexclude=<category>", strprintf("Exclude debugging information for a category. Can be used in conjunction with -debug=1 to output debug logs for all categories except one or more specified categories."), false, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-logips", strprintf("Include IP addresses in debug output (default: %u)", DEFAULT_LOGIPS), false, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-logtimestamps", strprintf("Prepend debug output with timestamp (default: %u)", DEFAULT_LOGTIMESTAMPS), false, OptionsCategory::DEBUG_TEST);
+ gArgs.AddArg("-logthreadnames", strprintf("Prepend debug output with name of the originating thread (only available on platforms supporting thread_local) (default: %u)", DEFAULT_LOGTHREADNAMES), false, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-logtimemicros", strprintf("Add microsecond precision to debug timestamps (default: %u)", DEFAULT_LOGTIMEMICROS), true, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-mocktime=<n>", "Replace actual time with <n> seconds since epoch (default: 0)", true, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-maxsigcachesize=<n>", strprintf("Limit sum of signature cache and script execution cache sizes to <n> MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_SIZE), true, OptionsCategory::DEBUG_TEST);
@@ -866,6 +867,7 @@ void InitLogging()
LogInstance().m_print_to_console = gArgs.GetBoolArg("-printtoconsole", !gArgs.GetBoolArg("-daemon", false));
LogInstance().m_log_timestamps = gArgs.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS);
LogInstance().m_log_time_micros = gArgs.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS);
+ LogInstance().m_log_threadnames = gArgs.GetBoolArg("-logthreadnames", DEFAULT_LOGTHREADNAMES);
fLogIPs = gArgs.GetBoolArg("-logips", DEFAULT_LOGIPS);
diff --git a/src/logging.cpp b/src/logging.cpp
index 36cad6573..3eda4995d 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -4,8 +4,11 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <logging.h>
+#include <util/threadnames.h>
#include <util/time.h>
+#include <mutex>
+
const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
BCLog::Logger& LogInstance()
@@ -174,7 +177,7 @@ std::vector<CLogCategoryActive> ListActiveLogCategories()
return ret;
}
-std::string BCLog::Logger::LogTimestampStr(const std::string &str)
+std::string BCLog::Logger::LogTimestampStr(const std::string& str)
{
std::string strStamped;
@@ -196,21 +199,24 @@ std::string BCLog::Logger::LogTimestampStr(const std::string &str)
} else
strStamped = str;
- if (!str.empty() && str[str.size()-1] == '\n')
- m_started_new_line = true;
- else
- m_started_new_line = false;
-
return strStamped;
}
void BCLog::Logger::LogPrintStr(const std::string &str)
{
- std::string strTimestamped = LogTimestampStr(str);
+ std::string str_prefixed = str;
+
+ if (m_log_threadnames && m_started_new_line) {
+ str_prefixed.insert(0, "[" + util::ThreadGetInternalName() + "] ");
+ }
+
+ str_prefixed = LogTimestampStr(str_prefixed);
+
+ m_started_new_line = !str.empty() && str[str.size()-1] == '\n';
if (m_print_to_console) {
// print to console
- fwrite(strTimestamped.data(), 1, strTimestamped.size(), stdout);
+ fwrite(str_prefixed.data(), 1, str_prefixed.size(), stdout);
fflush(stdout);
}
if (m_print_to_file) {
@@ -218,7 +224,7 @@ void BCLog::Logger::LogPrintStr(const std::string &str)
// buffer if we haven't opened the log yet
if (m_fileout == nullptr) {
- m_msgs_before_open.push_back(strTimestamped);
+ m_msgs_before_open.push_back(str_prefixed);
}
else
{
@@ -232,7 +238,7 @@ void BCLog::Logger::LogPrintStr(const std::string &str)
m_fileout = new_fileout;
}
}
- FileWriteStr(strTimestamped, m_fileout);
+ FileWriteStr(str_prefixed, m_fileout);
}
}
}
diff --git a/src/logging.h b/src/logging.h
index ac9d0dc0c..e399d4c30 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -19,6 +19,7 @@
static const bool DEFAULT_LOGTIMEMICROS = false;
static const bool DEFAULT_LOGIPS = false;
static const bool DEFAULT_LOGTIMESTAMPS = true;
+static const bool DEFAULT_LOGTHREADNAMES = false;
extern const char * const DEFAULT_DEBUGLOGFILE;
extern bool fLogIPs;
@@ -81,6 +82,7 @@ namespace BCLog {
bool m_log_timestamps = DEFAULT_LOGTIMESTAMPS;
bool m_log_time_micros = DEFAULT_LOGTIMEMICROS;
+ bool m_log_threadnames = DEFAULT_LOGTHREADNAMES;
fs::path m_file_path;
std::atomic<bool> m_reopen_file{false};
From 8722e54e56fd959fd4ff2321b36a7640dee440c5 Mon Sep 17 00:00:00 2001
From: James O'Beirne <james.obeirne@gmail.com>
Date: Sat, 5 May 2018 14:15:30 -0400
Subject: [PATCH 5/5] threads: add thread names to deadlock debugging message
Also refactor CLockLocation to use an initialization list.
---
src/sync.cpp | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/sync.cpp b/src/sync.cpp
index 23ca866e5..e7c0a6f9b 100644
--- a/src/sync.cpp
+++ b/src/sync.cpp
@@ -3,9 +3,11 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <sync.h>
+#include <tinyformat.h>
#include <logging.h>
#include <util/strencodings.h>
+#include <util/threadnames.h>
#include <stdio.h>
@@ -37,23 +39,30 @@ void PrintLockContention(const char* pszName, const char* pszFile, int nLine)
//
struct CLockLocation {
- CLockLocation(const char* pszName, const char* pszFile, int nLine, bool fTryIn)
- {
- mutexName = pszName;
- sourceFile = pszFile;
- sourceLine = nLine;
- fTry = fTryIn;
- }
+ CLockLocation(
+ const char* pszName,
+ const char* pszFile,
+ int nLine,
+ bool fTryIn,
+ const std::string& thread_name)
+ : fTry(fTryIn),
+ mutexName(pszName),
+ sourceFile(pszFile),
+ m_thread_name(thread_name),
+ sourceLine(nLine) {}
std::string ToString() const
{
- return mutexName + " " + sourceFile + ":" + itostr(sourceLine) + (fTry ? " (TRY)" : "");
+ return tfm::format(
+ "%s %s:%s%s (in thread %s)",
+ mutexName, sourceFile, itostr(sourceLine), (fTry ? " (TRY)" : ""), m_thread_name);
}
private:
bool fTry;
std::string mutexName;
std::string sourceFile;
+ const std::string& m_thread_name;
int sourceLine;
};
@@ -125,7 +134,7 @@ static void push_lock(void* c, const CLockLocation& locklocation)
std::pair<void*, void*> p1 = std::make_pair(i.first, c);
if (lockdata.lockorders.count(p1))
continue;
- lockdata.lockorders[p1] = g_lockstack;
+ lockdata.lockorders.emplace(p1, g_lockstack);
std::pair<void*, void*> p2 = std::make_pair(c, i.first);
lockdata.invlockorders.insert(p2);
@@ -141,7 +150,7 @@ static void pop_lock()
void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry)
{
- push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry));
+ push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry, util::ThreadGetInternalName()));
}
void LeaveCritical()