From c59abe25892e32c803ec527c40e9a74ab31ea58a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 10 May 2012 18:44:07 +0200 Subject: [PATCH 1/3] Use semaphores instead of condition variables --- src/net.cpp | 66 +++++++++++++++++++++++++------------------------ src/net.h | 1 + src/util.h | 71 ++++++++++++++++++++++++++++++++++++++++++----------- 3 files changed, 91 insertions(+), 47 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 8603514f..67427a3e 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -35,7 +35,7 @@ void ThreadOpenAddedConnections2(void* parg); void ThreadMapPort2(void* parg); #endif void ThreadDNSAddressSeed2(void* parg); -bool OpenNetworkConnection(const CAddress& addrConnect, const char *strDest = NULL, bool fOneShot = false); +bool OpenNetworkConnection(const CAddress& addrConnect, CSemaphoreGrant *grantOutbound = NULL, const char *strDest = NULL, bool fOneShot = false); @@ -66,10 +66,7 @@ CCriticalSection cs_vOneShots; set setservAddNodeAddresses; CCriticalSection cs_setservAddNodeAddresses; -static CWaitableCriticalSection csOutbound; -static int nOutbound = 0; -static CConditionVariable condOutbound; - +static CSemaphore *semOutbound = NULL; void AddOneShot(string strDest) { @@ -463,10 +460,6 @@ CNode* ConnectNode(CAddress addrConnect, const char *pszDest, int64 nTimeout) LOCK(cs_vNodes); vNodes.push_back(pnode); } - { - WAITABLE_LOCK(csOutbound); - nOutbound++; - } pnode->nTimeConnected = GetTime(); return pnode; @@ -612,14 +605,8 @@ void ThreadSocketHandler2(void* parg) // remove from vNodes vNodes.erase(remove(vNodes.begin(), vNodes.end(), pnode), vNodes.end()); - if (!pnode->fInbound) - { - WAITABLE_LOCK(csOutbound); - nOutbound--; - - // Connection slot(s) were removed, notify connection creator(s) - NOTIFY(condOutbound); - } + // release outbound grant (if any) + pnode->grantOutbound.Release(); // close socket and cleanup pnode->CloseSocketDisconnect(); @@ -1295,8 +1282,11 @@ void static ProcessOneShot() vOneShots.pop_front(); } CAddress addr; - if (!OpenNetworkConnection(addr, strDest.c_str(), true)) - AddOneShot(strDest); + CSemaphoreGrant grant(*semOutbound, true); + if (grant) { + if (!OpenNetworkConnection(addr, &grant, strDest.c_str(), true)) + AddOneShot(strDest); + } } void ThreadOpenConnections2(void* parg) @@ -1312,7 +1302,7 @@ void ThreadOpenConnections2(void* parg) BOOST_FOREACH(string strAddr, mapMultiArgs["-connect"]) { CAddress addr; - OpenNetworkConnection(addr, strAddr.c_str()); + OpenNetworkConnection(addr, NULL, strAddr.c_str()); for (int i = 0; i < 10 && i < nLoop; i++) { Sleep(500); @@ -1335,13 +1325,9 @@ void ThreadOpenConnections2(void* parg) if (fShutdown) return; - // Limit outbound connections - int nMaxOutbound = min(MAX_OUTBOUND_CONNECTIONS, (int)GetArg("-maxconnections", 125)); + vnThreadsRunning[THREAD_OPENCONNECTIONS]--; - { - WAITABLE_LOCK(csOutbound); - WAIT(condOutbound, fShutdown || nOutbound < nMaxOutbound); - } + CSemaphoreGrant grant(*semOutbound); vnThreadsRunning[THREAD_OPENCONNECTIONS]++; if (fShutdown) return; @@ -1374,11 +1360,15 @@ void ThreadOpenConnections2(void* parg) // Only connect to one address per a.b.?.? range. // Do this here so we don't have to critsect vNodes inside mapAddresses critsect. + int nOutbound = 0; set > setConnected; { LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodes) + BOOST_FOREACH(CNode* pnode, vNodes) { setConnected.insert(pnode->addr.GetGroup()); + if (!pnode->fInbound) + nOutbound++; + } } int64 nANow = GetAdjustedTime(); @@ -1408,7 +1398,7 @@ void ThreadOpenConnections2(void* parg) } if (addrConnect.IsValid()) - OpenNetworkConnection(addrConnect); + OpenNetworkConnection(addrConnect, &grant); } } @@ -1442,7 +1432,8 @@ void ThreadOpenAddedConnections2(void* parg) while(!fShutdown) { BOOST_FOREACH(string& strAddNode, mapMultiArgs["-addnode"]) { CAddress addr; - OpenNetworkConnection(addr, strAddNode.c_str()); + CSemaphoreGrant grant(*semOutbound); + OpenNetworkConnection(addr, &grant, strAddNode.c_str()); Sleep(500); } vnThreadsRunning[THREAD_ADDEDCONNECTIONS]--; @@ -1485,7 +1476,8 @@ void ThreadOpenAddedConnections2(void* parg) } BOOST_FOREACH(vector& vserv, vservConnectAddresses) { - OpenNetworkConnection(CAddress(*(vserv.begin()))); + CSemaphoreGrant grant(*semOutbound); + OpenNetworkConnection(CAddress(*(vserv.begin())), &grant); Sleep(500); if (fShutdown) return; @@ -1500,7 +1492,8 @@ void ThreadOpenAddedConnections2(void* parg) } } -bool OpenNetworkConnection(const CAddress& addrConnect, const char *strDest, bool fOneShot) +// if succesful, this moves the passed grant to the constructed node +bool OpenNetworkConnection(const CAddress& addrConnect, CSemaphoreGrant *grantOutbound, const char *strDest, bool fOneShot) { // // Initiate outbound network connection @@ -1522,6 +1515,8 @@ bool OpenNetworkConnection(const CAddress& addrConnect, const char *strDest, boo return false; if (!pnode) return false; + if (grantOutbound) + grantOutbound->MoveTo(pnode->grantOutbound); pnode->fNetworkNode = true; if (fOneShot) pnode->fOneShot = true; @@ -1770,6 +1765,12 @@ void StartNode(void* parg) #endif #endif + if (semOutbound == NULL) { + // initialize semaphore + int nMaxOutbound = min(MAX_OUTBOUND_CONNECTIONS, (int)GetArg("-maxconnections", 125)); + semOutbound = new CSemaphore(nMaxOutbound); + } + if (pnodeLocalHost == NULL) pnodeLocalHost = new CNode(INVALID_SOCKET, CAddress(CService("127.0.0.1", 0), nLocalServices)); @@ -1823,7 +1824,8 @@ bool StopNode() fShutdown = true; nTransactionsUpdated++; int64 nStart = GetTime(); - NOTIFY_ALL(condOutbound); + for (int i=0; ipost(); do { int nThreadsRunning = 0; diff --git a/src/net.h b/src/net.h index a00dd1b8..4e4ea31e 100644 --- a/src/net.h +++ b/src/net.h @@ -147,6 +147,7 @@ public: bool fNetworkNode; bool fSuccessfullyConnected; bool fDisconnect; + CSemaphoreGrant grantOutbound; protected: int nRefCount; diff --git a/src/util.h b/src/util.h index ebd574f8..61ff5535 100644 --- a/src/util.h +++ b/src/util.h @@ -23,7 +23,7 @@ typedef int pid_t; /* define for windows compatiblity */ #include #include #include -#include +#include #include #include #include @@ -275,24 +275,10 @@ public: }; typedef CMutexLock CCriticalBlock; -typedef CMutexLock CWaitableCriticalBlock; -typedef boost::interprocess::interprocess_condition CConditionVariable; - -/** Wait for a given condition inside a WAITABLE_CRITICAL_BLOCK */ -#define WAIT(name,condition) \ - do { while(!(condition)) { (name).wait(waitablecriticalblock.GetLock()); } } while(0) - -/** Notify waiting threads that a condition may hold now */ -#define NOTIFY(name) \ - do { (name).notify_one(); } while(0) - -#define NOTIFY_ALL(name) \ - do { (name).notify_all(); } while(0) #define LOCK(cs) CCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__) #define LOCK2(cs1,cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__),criticalblock2(cs2, #cs2, __FILE__, __LINE__) #define TRY_LOCK(cs,name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true) -#define WAITABLE_LOCK(cs) CWaitableCriticalBlock waitablecriticalblock(cs, #cs, __FILE__, __LINE__) #define ENTER_CRITICAL_SECTION(cs) \ { \ @@ -306,6 +292,61 @@ typedef boost::interprocess::interprocess_condition CConditionVariable; LeaveCritical(); \ } +typedef boost::interprocess::interprocess_semaphore CSemaphore; + +/** RAII-style semaphore lock */ +class CSemaphoreGrant +{ +private: + CSemaphore *sem; + bool fHaveGrant; + +public: + void Acquire() { + if (fHaveGrant) + return; + sem->wait(); + fHaveGrant = true; + } + + void Release() { + if (!fHaveGrant) + return; + sem->post(); + fHaveGrant = false; + } + + bool TryAcquire() { + if (!fHaveGrant && sem->try_wait()) + fHaveGrant = true; + return fHaveGrant; + } + + void MoveTo(CSemaphoreGrant &grant) { + grant.Release(); + grant.sem = sem; + grant.fHaveGrant = fHaveGrant; + sem = NULL; + fHaveGrant = false; + } + + CSemaphoreGrant() : sem(NULL), fHaveGrant(false) {} + + CSemaphoreGrant(CSemaphore &sema, bool fTry = false) : sem(&sema), fHaveGrant(false) { + if (fTry) + TryAcquire(); + else + Acquire(); + } + + ~CSemaphoreGrant() { + Release(); + } + + operator bool() { + return fHaveGrant; + } +}; inline std::string i64tostr(int64 n) { From 5456ef30926fa38b0d1705f9dcc6aa8def8a802d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 10 May 2012 20:45:35 +0200 Subject: [PATCH 2/3] Use polling instead of boost's broken semaphore on OSX --- src/util.h | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/util.h b/src/util.h index 61ff5535..f25a030f 100644 --- a/src/util.h +++ b/src/util.h @@ -292,7 +292,47 @@ typedef CMutexLock CCriticalBlock; LeaveCritical(); \ } +#ifdef MAC_OSX +// boost::interprocess::interprocess_semaphore seems to spinlock on OSX; prefer polling instead +class CSemaphore +{ +private: + CCriticalSection cs; + int val; + +public: + CSemaphore(int init) : val(init) {} + + void wait() { + do { + { + LOCK(cs); + if (val>0) { + val--; + return; + } + } + Sleep(100); + } while(1); + } + + bool try_wait() { + LOCK(cs); + if (val>0) { + val--; + return true; + } + return false; + } + + void post() { + LOCK(cs); + val++; + } +}; +#else typedef boost::interprocess::interprocess_semaphore CSemaphore; +#endif /** RAII-style semaphore lock */ class CSemaphoreGrant From 7f3ccb59da31c7b1706ebfbb401910923221f076 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 11 May 2012 17:00:03 +0200 Subject: [PATCH 3/3] Split synchronization mechanisms from util.{h,cpp} --- bitcoin-qt.pro | 2 + src/addrman.h | 1 + src/keystore.h | 2 +- src/main.h | 1 + src/makefile.linux-mingw | 1 + src/makefile.mingw | 1 + src/makefile.osx | 1 + src/makefile.unix | 1 + src/qt/messagepage.cpp | 1 - src/qt/qtipcserver.cpp | 1 - src/sync.cpp | 119 +++++++++++++++++++++ src/sync.h | 216 +++++++++++++++++++++++++++++++++++++++ src/util.cpp | 132 ++---------------------- src/util.h | 204 ------------------------------------ 14 files changed, 353 insertions(+), 330 deletions(-) create mode 100644 src/sync.cpp create mode 100644 src/sync.h diff --git a/bitcoin-qt.pro b/bitcoin-qt.pro index 112e8e93..c19fd4e0 100644 --- a/bitcoin-qt.pro +++ b/bitcoin-qt.pro @@ -109,6 +109,7 @@ HEADERS += src/qt/bitcoingui.h \ src/bignum.h \ src/checkpoints.h \ src/compat.h \ + src/sync.h \ src/util.h \ src/uint256.h \ src/serialize.h \ @@ -172,6 +173,7 @@ SOURCES += src/qt/bitcoin.cpp src/qt/bitcoingui.cpp \ src/qt/editaddressdialog.cpp \ src/qt/bitcoinaddressvalidator.cpp \ src/version.cpp \ + src/sync.cpp \ src/util.cpp \ src/netbase.cpp \ src/key.cpp \ diff --git a/src/addrman.h b/src/addrman.h index 43b6d35e..a1275da2 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -7,6 +7,7 @@ #include "netbase.h" #include "protocol.h" #include "util.h" +#include "sync.h" #include diff --git a/src/keystore.h b/src/keystore.h index 76820e20..52889b18 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -6,7 +6,7 @@ #define BITCOIN_KEYSTORE_H #include "crypter.h" -#include "util.h" +#include "sync.h" #include "base58.h" class CScript; diff --git a/src/main.h b/src/main.h index 194d9fdc..5ac5547a 100644 --- a/src/main.h +++ b/src/main.h @@ -6,6 +6,7 @@ #define BITCOIN_MAIN_H #include "bignum.h" +#include "sync.h" #include "net.h" #include "script.h" diff --git a/src/makefile.linux-mingw b/src/makefile.linux-mingw index 645f0a16..cc33bc0b 100644 --- a/src/makefile.linux-mingw +++ b/src/makefile.linux-mingw @@ -61,6 +61,7 @@ OBJS= \ obj/bitcoinrpc.o \ obj/rpcdump.o \ obj/script.o \ + obj/sync.o \ obj/util.o \ obj/wallet.o \ obj/walletdb.o \ diff --git a/src/makefile.mingw b/src/makefile.mingw index bb646695..27d27565 100644 --- a/src/makefile.mingw +++ b/src/makefile.mingw @@ -58,6 +58,7 @@ OBJS= \ obj/bitcoinrpc.o \ obj/rpcdump.o \ obj/script.o \ + obj/sync.o \ obj/util.o \ obj/wallet.o \ obj/walletdb.o \ diff --git a/src/makefile.osx b/src/makefile.osx index eb9ae4ba..b3afaa8e 100644 --- a/src/makefile.osx +++ b/src/makefile.osx @@ -85,6 +85,7 @@ OBJS= \ obj/bitcoinrpc.o \ obj/rpcdump.o \ obj/script.o \ + obj/sync.o \ obj/util.o \ obj/wallet.o \ obj/walletdb.o \ diff --git a/src/makefile.unix b/src/makefile.unix index 53fb1f0b..5cbf45e9 100644 --- a/src/makefile.unix +++ b/src/makefile.unix @@ -102,6 +102,7 @@ OBJS= \ obj/bitcoinrpc.o \ obj/rpcdump.o \ obj/script.o \ + obj/sync.o \ obj/util.o \ obj/wallet.o \ obj/walletdb.o \ diff --git a/src/qt/messagepage.cpp b/src/qt/messagepage.cpp index c04d8b2c..1f895e28 100644 --- a/src/qt/messagepage.cpp +++ b/src/qt/messagepage.cpp @@ -10,7 +10,6 @@ #include "main.h" #include "wallet.h" #include "init.h" -#include "util.h" #include "messagepage.h" #include "ui_messagepage.h" diff --git a/src/qt/qtipcserver.cpp b/src/qt/qtipcserver.cpp index 102ac0ff..06ada5aa 100644 --- a/src/qt/qtipcserver.cpp +++ b/src/qt/qtipcserver.cpp @@ -8,7 +8,6 @@ #include #include "ui_interface.h" -#include "util.h" #include "qtipcserver.h" using namespace boost::interprocess; diff --git a/src/sync.cpp b/src/sync.cpp new file mode 100644 index 00000000..fd9bcb62 --- /dev/null +++ b/src/sync.cpp @@ -0,0 +1,119 @@ +// Copyright (c) 2011-2012 The Bitcoin developers +// Distributed under the MIT/X11 software license, see the accompanying +// file license.txt or http://www.opensource.org/licenses/mit-license.php. + +#include "sync.h" + + + +#ifdef DEBUG_LOCKORDER +// +// Early deadlock detection. +// Problem being solved: +// Thread 1 locks A, then B, then C +// Thread 2 locks D, then C, then A +// --> may result in deadlock between the two threads, depending on when they run. +// Solution implemented here: +// Keep track of pairs of locks: (A before B), (A before C), etc. +// Complain if any thread trys to lock in a different order. +// + +struct CLockLocation +{ + CLockLocation(const char* pszName, const char* pszFile, int nLine) + { + mutexName = pszName; + sourceFile = pszFile; + sourceLine = nLine; + } + + std::string ToString() const + { + return mutexName+" "+sourceFile+":"+itostr(sourceLine); + } + +private: + std::string mutexName; + std::string sourceFile; + int sourceLine; +}; + +typedef std::vector< std::pair > LockStack; + +static boost::interprocess::interprocess_mutex dd_mutex; +static std::map, LockStack> lockorders; +static boost::thread_specific_ptr lockstack; + + +static void potential_deadlock_detected(const std::pair& mismatch, const LockStack& s1, const LockStack& s2) +{ + printf("POTENTIAL DEADLOCK DETECTED\n"); + printf("Previous lock order was:\n"); + BOOST_FOREACH(const PAIRTYPE(void*, CLockLocation)& i, s2) + { + if (i.first == mismatch.first) printf(" (1)"); + if (i.first == mismatch.second) printf(" (2)"); + printf(" %s\n", i.second.ToString().c_str()); + } + printf("Current lock order is:\n"); + BOOST_FOREACH(const PAIRTYPE(void*, CLockLocation)& i, s1) + { + if (i.first == mismatch.first) printf(" (1)"); + if (i.first == mismatch.second) printf(" (2)"); + printf(" %s\n", i.second.ToString().c_str()); + } +} + +static void push_lock(void* c, const CLockLocation& locklocation, bool fTry) +{ + bool fOrderOK = true; + if (lockstack.get() == NULL) + lockstack.reset(new LockStack); + + if (fDebug) printf("Locking: %s\n", locklocation.ToString().c_str()); + dd_mutex.lock(); + + (*lockstack).push_back(std::make_pair(c, locklocation)); + + if (!fTry) BOOST_FOREACH(const PAIRTYPE(void*, CLockLocation)& i, (*lockstack)) + { + if (i.first == c) break; + + std::pair p1 = std::make_pair(i.first, c); + if (lockorders.count(p1)) + continue; + lockorders[p1] = (*lockstack); + + std::pair p2 = std::make_pair(c, i.first); + if (lockorders.count(p2)) + { + potential_deadlock_detected(p1, lockorders[p2], lockorders[p1]); + break; + } + } + dd_mutex.unlock(); +} + +static void pop_lock() +{ + if (fDebug) + { + const CLockLocation& locklocation = (*lockstack).rbegin()->second; + printf("Unlocked: %s\n", locklocation.ToString().c_str()); + } + dd_mutex.lock(); + (*lockstack).pop_back(); + dd_mutex.unlock(); +} + +void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry) +{ + push_lock(cs, CLockLocation(pszName, pszFile, nLine), fTry); +} + +void LeaveCritical() +{ + pop_lock(); +} + +#endif /* DEBUG_LOCKORDER */ diff --git a/src/sync.h b/src/sync.h new file mode 100644 index 00000000..44d753ac --- /dev/null +++ b/src/sync.h @@ -0,0 +1,216 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2012 The Bitcoin developers +// Distributed under the MIT/X11 software license, see the accompanying +// file license.txt or http://www.opensource.org/licenses/mit-license.php. +#ifndef BITCOIN_SYNC_H +#define BITCOIN_SYNC_H + +#include +#include +#include +#include + + + + +/** Wrapped boost mutex: supports recursive locking, but no waiting */ +typedef boost::interprocess::interprocess_recursive_mutex CCriticalSection; + +/** Wrapped boost mutex: supports waiting but not recursive locking */ +typedef boost::interprocess::interprocess_mutex CWaitableCriticalSection; + +#ifdef DEBUG_LOCKORDER +void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); +void LeaveCritical(); +#else +void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} +void static inline LeaveCritical() {} +#endif + +/** Wrapper around boost::interprocess::scoped_lock */ +template +class CMutexLock +{ +private: + boost::interprocess::scoped_lock lock; +public: + + void Enter(const char* pszName, const char* pszFile, int nLine) + { + if (!lock.owns()) + { + EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex())); +#ifdef DEBUG_LOCKCONTENTION + if (!lock.try_lock()) + { + printf("LOCKCONTENTION: %s\n", pszName); + printf("Locker: %s:%d\n", pszFile, nLine); +#endif + lock.lock(); +#ifdef DEBUG_LOCKCONTENTION + } +#endif + } + } + + void Leave() + { + if (lock.owns()) + { + lock.unlock(); + LeaveCritical(); + } + } + + bool TryEnter(const char* pszName, const char* pszFile, int nLine) + { + if (!lock.owns()) + { + EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true); + lock.try_lock(); + if (!lock.owns()) + LeaveCritical(); + } + return lock.owns(); + } + + CMutexLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) : lock(mutexIn, boost::interprocess::defer_lock) + { + if (fTry) + TryEnter(pszName, pszFile, nLine); + else + Enter(pszName, pszFile, nLine); + } + + ~CMutexLock() + { + if (lock.owns()) + LeaveCritical(); + } + + operator bool() + { + return lock.owns(); + } + + boost::interprocess::scoped_lock &GetLock() + { + return lock; + } +}; + +typedef CMutexLock CCriticalBlock; + +#define LOCK(cs) CCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__) +#define LOCK2(cs1,cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__),criticalblock2(cs2, #cs2, __FILE__, __LINE__) +#define TRY_LOCK(cs,name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true) + +#define ENTER_CRITICAL_SECTION(cs) \ + { \ + EnterCritical(#cs, __FILE__, __LINE__, (void*)(&cs)); \ + (cs).lock(); \ + } + +#define LEAVE_CRITICAL_SECTION(cs) \ + { \ + (cs).unlock(); \ + LeaveCritical(); \ + } + +#ifdef MAC_OSX +// boost::interprocess::interprocess_semaphore seems to spinlock on OSX; prefer polling instead +class CSemaphore +{ +private: + CCriticalSection cs; + int val; + +public: + CSemaphore(int init) : val(init) {} + + void wait() { + do { + { + LOCK(cs); + if (val>0) { + val--; + return; + } + } + Sleep(100); + } while(1); + } + + bool try_wait() { + LOCK(cs); + if (val>0) { + val--; + return true; + } + return false; + } + + void post() { + LOCK(cs); + val++; + } +}; +#else +typedef boost::interprocess::interprocess_semaphore CSemaphore; +#endif + +/** RAII-style semaphore lock */ +class CSemaphoreGrant +{ +private: + CSemaphore *sem; + bool fHaveGrant; + +public: + void Acquire() { + if (fHaveGrant) + return; + sem->wait(); + fHaveGrant = true; + } + + void Release() { + if (!fHaveGrant) + return; + sem->post(); + fHaveGrant = false; + } + + bool TryAcquire() { + if (!fHaveGrant && sem->try_wait()) + fHaveGrant = true; + return fHaveGrant; + } + + void MoveTo(CSemaphoreGrant &grant) { + grant.Release(); + grant.sem = sem; + grant.fHaveGrant = fHaveGrant; + sem = NULL; + fHaveGrant = false; + } + + CSemaphoreGrant() : sem(NULL), fHaveGrant(false) {} + + CSemaphoreGrant(CSemaphore &sema, bool fTry = false) : sem(&sema), fHaveGrant(false) { + if (fTry) + TryAcquire(); + else + Acquire(); + } + + ~CSemaphoreGrant() { + Release(); + } + + operator bool() { + return fHaveGrant; + } +}; +#endif + diff --git a/src/util.cpp b/src/util.cpp index 1a57cc66..d2db7cec 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -4,6 +4,7 @@ // file license.txt or http://www.opensource.org/licenses/mit-license.php. #include "util.h" +#include "sync.h" #include "strlcpy.h" #include "version.h" #include "ui_interface.h" @@ -23,8 +24,6 @@ namespace boost { #include #include #include -#include -#include #include #include #include @@ -71,13 +70,14 @@ bool fLogTimestamps = false; CMedianFilter vTimeOffsets(200,0); // Init openssl library multithreading support -static boost::interprocess::interprocess_mutex** ppmutexOpenSSL; +static CCriticalSection** ppmutexOpenSSL; void locking_callback(int mode, int i, const char* file, int line) { - if (mode & CRYPTO_LOCK) - ppmutexOpenSSL[i]->lock(); - else - ppmutexOpenSSL[i]->unlock(); + if (mode & CRYPTO_LOCK) { + ENTER_CRITICAL_SECTION(*ppmutexOpenSSL[i]); + } else { + LEAVE_CRITICAL_SECTION(*ppmutexOpenSSL[i]); + } } // Init @@ -87,9 +87,9 @@ public: CInit() { // Init openssl library multithreading support - ppmutexOpenSSL = (boost::interprocess::interprocess_mutex**)OPENSSL_malloc(CRYPTO_num_locks() * sizeof(boost::interprocess::interprocess_mutex*)); + ppmutexOpenSSL = (CCriticalSection**)OPENSSL_malloc(CRYPTO_num_locks() * sizeof(CCriticalSection*)); for (int i = 0; i < CRYPTO_num_locks(); i++) - ppmutexOpenSSL[i] = new boost::interprocess::interprocess_mutex(); + ppmutexOpenSSL[i] = new CCriticalSection(); CRYPTO_set_locking_callback(locking_callback); #ifdef WIN32 @@ -1221,117 +1221,3 @@ bool GetStartOnSystemStartup() { return false; } bool SetStartOnSystemStartup(bool fAutoStart) { return false; } #endif - - - -#ifdef DEBUG_LOCKORDER -// -// Early deadlock detection. -// Problem being solved: -// Thread 1 locks A, then B, then C -// Thread 2 locks D, then C, then A -// --> may result in deadlock between the two threads, depending on when they run. -// Solution implemented here: -// Keep track of pairs of locks: (A before B), (A before C), etc. -// Complain if any thread trys to lock in a different order. -// - -struct CLockLocation -{ - CLockLocation(const char* pszName, const char* pszFile, int nLine) - { - mutexName = pszName; - sourceFile = pszFile; - sourceLine = nLine; - } - - std::string ToString() const - { - return mutexName+" "+sourceFile+":"+itostr(sourceLine); - } - -private: - std::string mutexName; - std::string sourceFile; - int sourceLine; -}; - -typedef std::vector< std::pair > LockStack; - -static boost::interprocess::interprocess_mutex dd_mutex; -static std::map, LockStack> lockorders; -static boost::thread_specific_ptr lockstack; - - -static void potential_deadlock_detected(const std::pair& mismatch, const LockStack& s1, const LockStack& s2) -{ - printf("POTENTIAL DEADLOCK DETECTED\n"); - printf("Previous lock order was:\n"); - BOOST_FOREACH(const PAIRTYPE(void*, CLockLocation)& i, s2) - { - if (i.first == mismatch.first) printf(" (1)"); - if (i.first == mismatch.second) printf(" (2)"); - printf(" %s\n", i.second.ToString().c_str()); - } - printf("Current lock order is:\n"); - BOOST_FOREACH(const PAIRTYPE(void*, CLockLocation)& i, s1) - { - if (i.first == mismatch.first) printf(" (1)"); - if (i.first == mismatch.second) printf(" (2)"); - printf(" %s\n", i.second.ToString().c_str()); - } -} - -static void push_lock(void* c, const CLockLocation& locklocation, bool fTry) -{ - bool fOrderOK = true; - if (lockstack.get() == NULL) - lockstack.reset(new LockStack); - - if (fDebug) printf("Locking: %s\n", locklocation.ToString().c_str()); - dd_mutex.lock(); - - (*lockstack).push_back(std::make_pair(c, locklocation)); - - if (!fTry) BOOST_FOREACH(const PAIRTYPE(void*, CLockLocation)& i, (*lockstack)) - { - if (i.first == c) break; - - std::pair p1 = std::make_pair(i.first, c); - if (lockorders.count(p1)) - continue; - lockorders[p1] = (*lockstack); - - std::pair p2 = std::make_pair(c, i.first); - if (lockorders.count(p2)) - { - potential_deadlock_detected(p1, lockorders[p2], lockorders[p1]); - break; - } - } - dd_mutex.unlock(); -} - -static void pop_lock() -{ - if (fDebug) - { - const CLockLocation& locklocation = (*lockstack).rbegin()->second; - printf("Unlocked: %s\n", locklocation.ToString().c_str()); - } - dd_mutex.lock(); - (*lockstack).pop_back(); - dd_mutex.unlock(); -} - -void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry) -{ - push_lock(cs, CLockLocation(pszName, pszFile, nLine), fTry); -} - -void LeaveCritical() -{ - pop_lock(); -} - -#endif /* DEBUG_LOCKORDER */ diff --git a/src/util.h b/src/util.h index f25a030f..1363fbbb 100644 --- a/src/util.h +++ b/src/util.h @@ -21,10 +21,6 @@ typedef int pid_t; /* define for windows compatiblity */ #include #include #include -#include -#include -#include -#include #include #include @@ -188,206 +184,6 @@ void AddTimeData(const CNetAddr& ip, int64 nTime); -/** Wrapped boost mutex: supports recursive locking, but no waiting */ -typedef boost::interprocess::interprocess_recursive_mutex CCriticalSection; - -/** Wrapped boost mutex: supports waiting but not recursive locking */ -typedef boost::interprocess::interprocess_mutex CWaitableCriticalSection; - -#ifdef DEBUG_LOCKORDER -void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); -void LeaveCritical(); -#else -void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} -void static inline LeaveCritical() {} -#endif - -/** Wrapper around boost::interprocess::scoped_lock */ -template -class CMutexLock -{ -private: - boost::interprocess::scoped_lock lock; -public: - - void Enter(const char* pszName, const char* pszFile, int nLine) - { - if (!lock.owns()) - { - EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex())); -#ifdef DEBUG_LOCKCONTENTION - if (!lock.try_lock()) - { - printf("LOCKCONTENTION: %s\n", pszName); - printf("Locker: %s:%d\n", pszFile, nLine); -#endif - lock.lock(); -#ifdef DEBUG_LOCKCONTENTION - } -#endif - } - } - - void Leave() - { - if (lock.owns()) - { - lock.unlock(); - LeaveCritical(); - } - } - - bool TryEnter(const char* pszName, const char* pszFile, int nLine) - { - if (!lock.owns()) - { - EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true); - lock.try_lock(); - if (!lock.owns()) - LeaveCritical(); - } - return lock.owns(); - } - - CMutexLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) : lock(mutexIn, boost::interprocess::defer_lock) - { - if (fTry) - TryEnter(pszName, pszFile, nLine); - else - Enter(pszName, pszFile, nLine); - } - - ~CMutexLock() - { - if (lock.owns()) - LeaveCritical(); - } - - operator bool() - { - return lock.owns(); - } - - boost::interprocess::scoped_lock &GetLock() - { - return lock; - } -}; - -typedef CMutexLock CCriticalBlock; - -#define LOCK(cs) CCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__) -#define LOCK2(cs1,cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__),criticalblock2(cs2, #cs2, __FILE__, __LINE__) -#define TRY_LOCK(cs,name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true) - -#define ENTER_CRITICAL_SECTION(cs) \ - { \ - EnterCritical(#cs, __FILE__, __LINE__, (void*)(&cs)); \ - (cs).lock(); \ - } - -#define LEAVE_CRITICAL_SECTION(cs) \ - { \ - (cs).unlock(); \ - LeaveCritical(); \ - } - -#ifdef MAC_OSX -// boost::interprocess::interprocess_semaphore seems to spinlock on OSX; prefer polling instead -class CSemaphore -{ -private: - CCriticalSection cs; - int val; - -public: - CSemaphore(int init) : val(init) {} - - void wait() { - do { - { - LOCK(cs); - if (val>0) { - val--; - return; - } - } - Sleep(100); - } while(1); - } - - bool try_wait() { - LOCK(cs); - if (val>0) { - val--; - return true; - } - return false; - } - - void post() { - LOCK(cs); - val++; - } -}; -#else -typedef boost::interprocess::interprocess_semaphore CSemaphore; -#endif - -/** RAII-style semaphore lock */ -class CSemaphoreGrant -{ -private: - CSemaphore *sem; - bool fHaveGrant; - -public: - void Acquire() { - if (fHaveGrant) - return; - sem->wait(); - fHaveGrant = true; - } - - void Release() { - if (!fHaveGrant) - return; - sem->post(); - fHaveGrant = false; - } - - bool TryAcquire() { - if (!fHaveGrant && sem->try_wait()) - fHaveGrant = true; - return fHaveGrant; - } - - void MoveTo(CSemaphoreGrant &grant) { - grant.Release(); - grant.sem = sem; - grant.fHaveGrant = fHaveGrant; - sem = NULL; - fHaveGrant = false; - } - - CSemaphoreGrant() : sem(NULL), fHaveGrant(false) {} - - CSemaphoreGrant(CSemaphore &sema, bool fTry = false) : sem(&sema), fHaveGrant(false) { - if (fTry) - TryAcquire(); - else - Acquire(); - } - - ~CSemaphoreGrant() { - Release(); - } - - operator bool() { - return fHaveGrant; - } -}; - inline std::string i64tostr(int64 n) { return strprintf("%"PRI64d, n);