From faffaa85cde32b621f598a8ea8dceae34f33f021 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 27 Sep 2021 13:11:30 +0200 Subject: [PATCH 1/4] log: Avoid broken SELECTCOINS log Before this patch, the log might be corrupted by other threads logging at the same time. For example, another RPC thread: [httpworker.1] [default wallet] keypool reserve 1296 [httpworker.1] SelectCoins() best subset: Received a POST request for / from 127.0.0.1:53732 [httpworker.3] ThreadRPCServer method=getnetworkinfo user=__cookie__ [httpworker.1] 0.78125 0.1953125 0.02441406 0.00610351 0.00305175 0.00152587 total 1.01025417 --- src/wallet/coinselection.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 1699424657c..8cff6e99b7f 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -279,13 +279,13 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector& group } if (LogAcceptCategory(BCLog::SELECTCOINS)) { - LogPrint(BCLog::SELECTCOINS, "SelectCoins() best subset: "); /* Continued */ + std::string log_message{"SelectCoins() best subset: "}; for (unsigned int i = 0; i < applicable_groups.size(); i++) { if (vfBest[i]) { - LogPrint(BCLog::SELECTCOINS, "%s ", FormatMoney(applicable_groups[i].m_value)); /* Continued */ + log_message += strprintf("%s ", FormatMoney(applicable_groups[i].m_value)); } } - LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); + LogPrint(BCLog::SELECTCOINS, "%stotal %s\n", log_message, FormatMoney(nBest)); } } From faeae2980fa2493391cdced20950a991e28cf47d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 27 Sep 2021 13:55:34 +0200 Subject: [PATCH 2/4] log: Avoid broken DEBUG_LOCKORDER log --- src/sync.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/sync.cpp b/src/sync.cpp index 98e6d3d65d8..c9fd8e347ed 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -97,27 +97,29 @@ static void potential_deadlock_detected(const LockPair& mismatch, const LockStac LogPrintf("POTENTIAL DEADLOCK DETECTED\n"); LogPrintf("Previous lock order was:\n"); for (const LockStackItem& i : s1) { + std::string prefix{}; if (i.first == mismatch.first) { - LogPrintf(" (1)"); /* Continued */ + prefix = " (1)"; } if (i.first == mismatch.second) { - LogPrintf(" (2)"); /* Continued */ + prefix = " (2)"; } - LogPrintf(" %s\n", i.second.ToString()); + LogPrintf("%s %s\n", prefix, i.second.ToString()); } std::string mutex_a, mutex_b; LogPrintf("Current lock order is:\n"); for (const LockStackItem& i : s2) { + std::string prefix{}; if (i.first == mismatch.first) { - LogPrintf(" (1)"); /* Continued */ + prefix = " (1)"; mutex_a = i.second.Name(); } if (i.first == mismatch.second) { - LogPrintf(" (2)"); /* Continued */ + prefix = " (2)"; mutex_b = i.second.Name(); } - LogPrintf(" %s\n", i.second.ToString()); + LogPrintf("%s %s\n", prefix, i.second.ToString()); } if (g_debug_lockorder_abort) { tfm::format(std::cerr, "Assertion failed: detected inconsistent lock order for %s, details in debug log.\n", s2.back().second.ToString()); @@ -131,10 +133,11 @@ static void double_lock_detected(const void* mutex, const LockStack& lock_stack) LogPrintf("DOUBLE LOCK DETECTED\n"); LogPrintf("Lock order:\n"); for (const LockStackItem& i : lock_stack) { + std::string prefix{}; if (i.first == mutex) { - LogPrintf(" (*)"); /* Continued */ + prefix = " (*)"; } - LogPrintf(" %s\n", i.second.ToString()); + LogPrintf("%s %s\n", prefix, i.second.ToString()); } if (g_debug_lockorder_abort) { tfm::format(std::cerr, From fa6c1e850f3a96f884ba8a635b72d3abea1f4e56 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 27 Sep 2021 14:25:00 +0200 Subject: [PATCH 3/4] test: Fix typos in tests --- test/functional/test_framework/util.py | 2 +- test/lint/lint-logs.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index ea5c641b4a2..f2637df35b8 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -366,7 +366,7 @@ def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect= f.write("listenonion=0\n") # Increase peertimeout to avoid disconnects while using mocktime. # peertimeout is measured in wall clock time, so setting it to the - # duration of the longest test is sufficient. It can be overriden in + # duration of the longest test is sufficient. It can be overridden in # tests. f.write("peertimeout=999999\n") f.write("printtoconsole=0\n") diff --git a/test/lint/lint-logs.sh b/test/lint/lint-logs.sh index 2fbb4a38e75..d6c53e8ff32 100755 --- a/test/lint/lint-logs.sh +++ b/test/lint/lint-logs.sh @@ -7,7 +7,7 @@ # Check that all logs are terminated with '\n' # # Some logs are continued over multiple lines. They should be explicitly -# commented with \* Continued *\ +# commented with /* Continued */ # # There are some instances of LogPrintf() in comments. Those can be # ignored From 2222c04e1b9960030cb590c789a0d2375add4544 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 28 Sep 2021 08:50:02 +0200 Subject: [PATCH 4/4] log: Adjust coin selection log string Replace the outdated function name with words from the English language. Logging the function name can be toggled with -logsourcelocations. --- src/wallet/coinselection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 8cff6e99b7f..a03d17cb7a7 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -279,7 +279,7 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector& group } if (LogAcceptCategory(BCLog::SELECTCOINS)) { - std::string log_message{"SelectCoins() best subset: "}; + std::string log_message{"Coin selection best subset: "}; for (unsigned int i = 0; i < applicable_groups.size(); i++) { if (vfBest[i]) { log_message += strprintf("%s ", FormatMoney(applicable_groups[i].m_value));