Merge pull request #3588 from patricklodder/1.14.8-fix-boost

1.14.8: enable compiling against newer boost versions

src/Makefile.test.include

@@ -127,6 +127,7 @@ BITCOIN_TESTS =\
   test/sigopcount_tests.cpp \
   test/skiplist_tests.cpp \
   test/streams_tests.cpp \
+  test/sync_tests.cpp \
   test/test_bitcoin.cpp \
   test/test_bitcoin.h \
   test/test_random.h \

src/bench/bench.cpp

@@ -3,11 +3,14 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include "bench.h"
-#include "perf.h"
 
-#include <iostream>
-#include <iomanip>
-#include <sys/time.h>
+#include <cstddef>     // for NULL
+#include <sys/time.h>  // for gettimeofday, timeval
+#include <iomanip>     // for operator<<, setprecision
+#include <iostream>    // for operator<<, basic_ostream
+#include <utility>     // for pair, make_pair
+
+#include "perf.h"      // for perf_cpucycles, perf_fini, perf_init
 
 benchmark::BenchRunner::BenchmarkMap &benchmark::BenchRunner::benchmarks() {
     static std::map<std::string, benchmark::BenchFunction> benchmarks_map;

src/bench/bench.h

@@ -5,12 +5,15 @@
 #ifndef BITCOIN_BENCH_BENCH_H
 #define BITCOIN_BENCH_BENCH_H
 
-#include <map>
-#include <string>
+#include <cstdint>                               // for uint64_t
 
-#include <boost/function.hpp>
-#include <boost/preprocessor/cat.hpp>
-#include <boost/preprocessor/stringize.hpp>
+#include <boost/function.hpp>                    // for function
+#include <boost/preprocessor/cat.hpp>            // for BOOST_PP_CAT
+#include <boost/preprocessor/stringize.hpp>      // for BOOST_PP_STRINGIZE
+
+#include <limits>                                // for numeric_limits
+#include <map>                                   // for map
+#include <string>                                // for string
 
 // Simple micro-benchmarking framework; API mostly matches a subset of the Google Benchmark
 // framework (see https://github.com/google/benchmark)

src/bench/bench_bitcoin.cpp

@@ -2,11 +2,9 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include "bench.h"
-
-#include "key.h"
-#include "validation.h"
-#include "util.h"
+#include "bench.h"  // for BenchRunner
+#include "key.h"    // for ECC_Start, ECC_Stop
+#include "util.h"   // for SetupEnvironment, fPrintToDebugLog
 
 int
 main(int argc, char** argv)

src/rpc/protocol.cpp

@@ -22,7 +22,7 @@ using namespace std;
  * JSON-RPC protocol.  Bitcoin speaks version 1.0 for maximum compatibility,
  * but uses JSON-RPC 1.1/2.0 standards for parts of the 1.0 standard that were
  * unspecified (HTTP errors and contents of 'error').
- * 
+ *
  * 1.0 spec: http://json-rpc.org/wiki/specification
  * 1.2 spec: http://jsonrpc.org/historical/json-rpc-over-http.html
  */
@@ -72,7 +72,7 @@ static const std::string COOKIEAUTH_FILE = ".cookie";
 boost::filesystem::path GetAuthCookieFile()
 {
     boost::filesystem::path path(GetArg("-rpccookiefile", COOKIEAUTH_FILE));
-    if (!path.is_complete()) path = GetDataDir() / path;
+    if (!path.is_absolute()) path = GetDataDir() / path;
     return path;
 }
 
@@ -126,4 +126,3 @@ void DeleteAuthCookie()
         LogPrintf("%s: Unable to remove random auth cookie file: %s\n", __func__, e.what());
     }
 }
-

src/sync.cpp

@@ -9,8 +9,13 @@
 
 #include <stdio.h>
 
-#include <boost/foreach.hpp>
-#include <boost/thread.hpp>
+#include <thread>
+#include <unordered_map>
+#include <utility>
+#include <vector>
+#include <set>
+#include <map>
+#include <mutex>
 
 #ifdef DEBUG_LOCKCONTENTION
 void PrintLockContention(const char* pszName, const char* pszFile, int nLine)
@@ -55,31 +60,31 @@ private:
     int sourceLine;
 };
 
-typedef std::vector<std::pair<void*, CLockLocation> > LockStack;
-typedef std::map<std::pair<void*, void*>, LockStack> LockOrders;
-typedef std::set<std::pair<void*, void*> > InvLockOrders;
+typedef std::pair<void*, CLockLocation> LockStackItem;
+typedef std::vector<LockStackItem> LockStack;
+typedef std::unordered_map<std::thread::id, LockStack> LockStacks;
+
+typedef std::pair<void*, void*> LockPair;
+typedef std::map<LockPair, LockStack> LockOrders;
+typedef std::set<LockPair> InvLockOrders;
 
 struct LockData {
-    // Very ugly hack: as the global constructs and destructors run single
-    // threaded, we use this boolean to know whether LockData still exists,
-    // as DeleteLock can get called by global CCriticalSection destructors
-    // after LockData disappears.
-    bool available;
-    LockData() : available(true) {}
-    ~LockData() { available = false; }
-
+    LockStacks m_lock_stacks;
     LockOrders lockorders;
     InvLockOrders invlockorders;
-    boost::mutex dd_mutex;
-} static lockdata;
+    std::mutex dd_mutex;
+};
 
-boost::thread_specific_ptr<LockStack> lockstack;
+LockData& GetLockData() {
+    static LockData& lock_data = *new LockData();
+    return lock_data;
+}
 
-static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch, const LockStack& s1, const LockStack& s2)
+static void potential_deadlock_detected(const LockPair& mismatch, const LockStack& s1, const LockStack& s2)
 {
     LogPrintf("POTENTIAL DEADLOCK DETECTED\n");
     LogPrintf("Previous lock order was:\n");
-    BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, s2) {
+    for (const LockStackItem& i : s2) {
         if (i.first == mismatch.first) {
             LogPrintf(" (1)");
         }
@@ -89,7 +94,7 @@ static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch,
         LogPrintf(" %s\n", i.second.ToString());
     }
     LogPrintf("Current lock order is:\n");
-    BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, s1) {
+    for (const LockStackItem& i : s1) {
         if (i.first == mismatch.first) {
             LogPrintf(" (1)");
         }
@@ -98,28 +103,31 @@ static void potential_deadlock_detected(const std::pair<void*, void*>& mismatch,
         }
         LogPrintf(" %s\n", i.second.ToString());
     }
-    assert(false);
+    if (g_debug_lockorder_abort) {
+        fprintf(stderr, "Assertion failed: detected inconsistent lock order at %s:%i, details in debug log.\n", __FILE__, __LINE__);
+        abort();
+    }
+    throw std::logic_error("potential deadlock detected");
 }
 
 static void push_lock(void* c, const CLockLocation& locklocation, bool fTry)
 {
-    if (lockstack.get() == NULL)
-        lockstack.reset(new LockStack);
+    LockData& lockdata = GetLockData();
+    std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
 
-    boost::unique_lock<boost::mutex> lock(lockdata.dd_mutex);
+    LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()];
+    lock_stack.emplace_back(c, locklocation);
 
-    (*lockstack).push_back(std::make_pair(c, locklocation));
-
-    BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, (*lockstack)) {
+    for (const LockStackItem& i : lock_stack) {
         if (i.first == c)
             break;
 
-        std::pair<void*, void*> p1 = std::make_pair(i.first, c);
+        const LockPair p1 = std::make_pair(i.first, c);
         if (lockdata.lockorders.count(p1))
             continue;
-        lockdata.lockorders[p1] = (*lockstack);
+        lockdata.lockorders[p1] = lock_stack;
 
-        std::pair<void*, void*> p2 = std::make_pair(c, i.first);
+        const LockPair p2 = std::make_pair(c, i.first);
         lockdata.invlockorders.insert(p2);
         if (lockdata.lockorders.count(p2))
             potential_deadlock_detected(p1, lockdata.lockorders[p2], lockdata.lockorders[p1]);
@@ -128,7 +136,14 @@ static void push_lock(void* c, const CLockLocation& locklocation, bool fTry)
 
 static void pop_lock()
 {
-    (*lockstack).pop_back();
+    LockData& lockdata = GetLockData();
+    std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
+
+    LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()];
+    lock_stack.pop_back();
+    if (lock_stack.empty()) {
+        lockdata.m_lock_stacks.erase(std::this_thread::get_id());
+    }
 }
 
 void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry)
@@ -143,41 +158,58 @@ void LeaveCritical()
 
 std::string LocksHeld()
 {
+    LockData& lockdata = GetLockData();
+    std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
+
+    const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()];
+
     std::string result;
-    BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, *lockstack)
+    for (const LockStackItem& i : lock_stack) {
         result += i.second.ToString() + std::string("\n");
+    }
     return result;
 }
 
+static bool LockHeld(void* mutex)
+{
+    LockData& lockdata = GetLockData();
+    std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
+
+    const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()];
+    for (const LockStackItem& i : lock_stack) {
+        if (i.first == mutex) return true;
+    }
+
+    return false;
+}
+
 void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs)
 {
-    BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, *lockstack)
-        if (i.first == cs)
-            return;
+    if (LockHeld(cs)) return;
     fprintf(stderr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld().c_str());
     abort();
 }
 
 void DeleteLock(void* cs)
 {
-    if (!lockdata.available) {
-        // We're already shutting down.
-        return;
-    }
-    boost::unique_lock<boost::mutex> lock(lockdata.dd_mutex);
-    std::pair<void*, void*> item = std::make_pair(cs, (void*)0);
+    LockData& lockdata = GetLockData();
+    std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
+
+    const LockPair item = std::make_pair(cs, (void*)0);
     LockOrders::iterator it = lockdata.lockorders.lower_bound(item);
     while (it != lockdata.lockorders.end() && it->first.first == cs) {
-        std::pair<void*, void*> invitem = std::make_pair(it->first.second, it->first.first);
+        const LockPair invitem = std::make_pair(it->first.second, it->first.first);
         lockdata.invlockorders.erase(invitem);
         lockdata.lockorders.erase(it++);
     }
     InvLockOrders::iterator invit = lockdata.invlockorders.lower_bound(item);
     while (invit != lockdata.invlockorders.end() && invit->first == cs) {
-        std::pair<void*, void*> invinvitem = std::make_pair(invit->second, invit->first);
+        const LockPair invinvitem = std::make_pair(invit->second, invit->first);
         lockdata.lockorders.erase(invinvitem);
         lockdata.invlockorders.erase(invit++);
     }
 }
 
+bool g_debug_lockorder_abort = true;
+
 #endif /* DEBUG_LOCKORDER */

src/sync.h

@@ -77,6 +77,13 @@ void LeaveCritical();
 std::string LocksHeld();
 void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs);
 void DeleteLock(void* cs);
+
+/**
+ * Call abort() if a potential lock order deadlock bug is detected, instead of
+ * just logging information and throwing a logic_error. Defaults to true, and
+ * set to false in DEBUG_LOCKORDER unit tests.
+ */
+extern bool g_debug_lockorder_abort;
 #else
 void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {}
 void static inline LeaveCritical() {}

src/test/sync_tests.cpp

@@ -0,0 +1,41 @@
+// Copyright (c) 2012-2017 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 <sync.h>
+#include <test/test_bitcoin.h>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
+{
+    #ifdef DEBUG_LOCKORDER
+    bool prev = g_debug_lockorder_abort;
+    g_debug_lockorder_abort = false;
+    #endif
+
+    CCriticalSection mutex1, mutex2;
+    {
+        LOCK2(mutex1, mutex2);
+    }
+    bool error_thrown = false;
+    try {
+        LOCK2(mutex2, mutex1);
+    } catch (const std::logic_error& e) {
+        BOOST_CHECK_EQUAL(e.what(), "potential deadlock detected");
+        error_thrown = true;
+    }
+    #ifdef DEBUG_LOCKORDER
+    BOOST_CHECK(error_thrown);
+    #else
+    BOOST_CHECK(!error_thrown);
+    #endif
+
+    #ifdef DEBUG_LOCKORDER
+    g_debug_lockorder_abort = prev;
+    #endif
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/util.cpp

@@ -590,7 +590,7 @@ const boost::filesystem::path &GetBackupDir()
 boost::filesystem::path GetConfigFile(const std::string& confPath)
 {
     boost::filesystem::path pathConfigFile(confPath);
-    if (!pathConfigFile.is_complete())
+    if (!pathConfigFile.is_absolute())
         pathConfigFile = GetDataDir(false) / pathConfigFile;
 
     return pathConfigFile;
@@ -626,7 +626,7 @@ void ReadConfigFile(const std::string& confPath)
 boost::filesystem::path GetPidFile()
 {
     boost::filesystem::path pathPidFile(GetArg("-pid", BITCOIN_PID_FILENAME));
-    if (!pathPidFile.is_complete()) pathPidFile = GetDataDir() / pathPidFile;
+    if (!pathPidFile.is_absolute()) pathPidFile = GetDataDir() / pathPidFile;
     return pathPidFile;
 }
 

src/validation.h

@@ -22,6 +22,7 @@
 
 #include <algorithm>
 #include <exception>
+#include <list>
 #include <map>
 #include <set>
 #include <stdint.h>

src/wallet/wallet.cpp

@@ -459,8 +459,10 @@ bool CWallet::Verify()
     uiInterface.InitMessage(_("Verifying wallet..."));
 
     // Wallet file must be a plain filename without a directory
-    if (walletFile != boost::filesystem::basename(walletFile) + boost::filesystem::extension(walletFile))
+    boost::filesystem::path walletPath(walletFile);
+    if (walletFile != walletPath.stem().string() + walletPath.extension().string()) {
         return InitError(strprintf(_("Wallet %s resides outside data directory %s"), walletFile, GetDataDir().string()));
+    }
 
     if (!bitdb.Open(GetDataDir()))
     {
@@ -4018,7 +4020,11 @@ bool CWallet::BackupWallet(const std::string& strDest)
                         return false;
                     }
 
-#if BOOST_VERSION >= 104000
+#if BOOST_VERSION >= 107400
+                    // Boost 1.74.0 and up implements std++17 like "copy_options", and this
+                    // is the only remaining enum after 1.85.0
+                    boost::filesystem::copy_file(pathSrc, pathDest, boost::filesystem::copy_options::overwrite_existing);
+#elif BOOST_VERSION >= 104000
                     boost::filesystem::copy_file(pathSrc, pathDest, boost::filesystem::copy_option::overwrite_if_exists);
 #else
                     boost::filesystem::copy_file(pathSrc, pathDest);