Skip to content

Commit 712fd18

Browse files
committed
Locking system overhaul, add condition variables
This commit simplifies the locking system: CCriticalSection becomes a simple typedef for boost::interprocess::interprocess_recursive_mutex, and CCriticalBlock and CTryCriticalBlock are replaced by a templated CMutexLock, which wraps boost::interprocess::scoped_lock. By making the lock type a template parameter, some critical sections can now be changed to non-recursive locks, which support waiting via condition variables. These are implemented in CWaitableCriticalSection and WAITABLE_CRITICAL_BLOCK. CWaitableCriticalSection is a wrapper for a different Boost mutex, which supports waiting/notification via condition variables. This should enable us to remove much of the used polling code. Important is that this mutex is not recursive, so functions that perform the locking must not call eachother. Because boost::interprocess::scoped_lock does not support assigning and copying, I had to revert to the older CRITICAL_BLOCK macros that use a nested for loop instead of a simple if.
1 parent b0a7e05 commit 712fd18

File tree

2 files changed

+110
-103
lines changed

2 files changed

+110
-103
lines changed

src/util.cpp

Lines changed: 4 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,62 +1193,14 @@ static void pop_lock()
11931193
dd_mutex.unlock();
11941194
}
11951195

1196-
void CCriticalSection::Enter(const char* pszName, const char* pszFile, int nLine)
1196+
void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs)
11971197
{
1198-
push_lock(this, CLockLocation(pszName, pszFile, nLine));
1199-
#ifdef DEBUG_LOCKCONTENTION
1200-
bool result = mutex.try_lock();
1201-
if (!result)
1202-
{
1203-
printf("LOCKCONTENTION: %s\n", pszName);
1204-
printf("Locker: %s:%d\n", pszFile, nLine);
1205-
mutex.lock();
1206-
printf("Locked\n");
1207-
}
1208-
#else
1209-
mutex.lock();
1210-
#endif
1211-
}
1212-
void CCriticalSection::Leave()
1213-
{
1214-
mutex.unlock();
1215-
pop_lock();
1216-
}
1217-
bool CCriticalSection::TryEnter(const char* pszName, const char* pszFile, int nLine)
1218-
{
1219-
push_lock(this, CLockLocation(pszName, pszFile, nLine));
1220-
bool result = mutex.try_lock();
1221-
if (!result) pop_lock();
1222-
return result;
1198+
push_lock(cs, CLockLocation(pszName, pszFile, nLine));
12231199
}
12241200

1225-
#else
1226-
1227-
void CCriticalSection::Enter(const char* pszName, const char* pszFile, int nLine)
1201+
void LeaveCritical()
12281202
{
1229-
#ifdef DEBUG_LOCKCONTENTION
1230-
bool result = mutex.try_lock();
1231-
if (!result)
1232-
{
1233-
printf("LOCKCONTENTION: %s\n", pszName);
1234-
printf("Locker: %s:%d\n", pszFile, nLine);
1235-
mutex.lock();
1236-
}
1237-
#else
1238-
mutex.lock();
1239-
#endif
1240-
}
1241-
1242-
void CCriticalSection::Leave()
1243-
{
1244-
mutex.unlock();
1245-
}
1246-
1247-
bool CCriticalSection::TryEnter(const char*, const char*, int)
1248-
{
1249-
bool result = mutex.try_lock();
1250-
return result;
1203+
pop_lock();
12511204
}
12521205

12531206
#endif /* DEBUG_LOCKORDER */
1254-

src/util.h

Lines changed: 106 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ typedef int pid_t; /* define for windows compatiblity */
2020

2121
#include <boost/thread.hpp>
2222
#include <boost/interprocess/sync/interprocess_recursive_mutex.hpp>
23+
#include <boost/interprocess/sync/scoped_lock.hpp>
24+
#include <boost/interprocess/sync/interprocess_condition.hpp>
25+
#include <boost/interprocess/sync/lock_options.hpp>
2326
#include <boost/date_time/gregorian/gregorian_types.hpp>
2427
#include <boost/date_time/posix_time/posix_time_types.hpp>
2528

@@ -180,82 +183,134 @@ void AddTimeData(const CNetAddr& ip, int64 nTime);
180183

181184

182185

186+
/** Wrapped boost mutex: supports recursive locking, but no waiting */
187+
typedef boost::interprocess::interprocess_recursive_mutex CCriticalSection;
183188

184-
/** Wrapper to automatically initialize mutex. */
185-
class CCriticalSection
186-
{
187-
protected:
188-
boost::interprocess::interprocess_recursive_mutex mutex;
189-
public:
190-
explicit CCriticalSection() { }
191-
~CCriticalSection() { }
192-
void Enter(const char* pszName, const char* pszFile, int nLine);
193-
void Leave();
194-
bool TryEnter(const char* pszName, const char* pszFile, int nLine);
195-
};
189+
/** Wrapped boost mutex: supports waiting but not recursive locking */
190+
typedef boost::interprocess::interprocess_mutex CWaitableCriticalSection;
196191

197-
/** RAII object that acquires mutex. Needed for exception safety. */
198-
class CCriticalBlock
199-
{
200-
protected:
201-
CCriticalSection* pcs;
192+
#ifdef DEBUG_LOCKORDER
193+
void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs);
194+
void LeaveCritical();
195+
#else
196+
void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs) {}
197+
void static inline LeaveCritical() {}
198+
#endif
202199

200+
/** Wrapper around boost::interprocess::scoped_lock */
201+
template<typename Mutex>
202+
class CMutexLock
203+
{
204+
private:
205+
boost::interprocess::scoped_lock<Mutex> lock;
203206
public:
204-
CCriticalBlock(CCriticalSection& csIn, const char* pszName, const char* pszFile, int nLine)
207+
208+
void Enter(const char* pszName, const char* pszFile, int nLine)
205209
{
206-
pcs = &csIn;
207-
pcs->Enter(pszName, pszFile, nLine);
210+
if (!lock.owns())
211+
{
212+
EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()));
213+
#ifdef DEBUG_LOCKCONTENTION
214+
if (!lock.try_lock())
215+
{
216+
printf("LOCKCONTENTION: %s\n", pszName);
217+
printf("Locker: %s:%d\n", pszFile, nLine);
218+
}
219+
#endif
220+
lock.lock();
221+
}
208222
}
209223

210-
operator bool() const
224+
void Leave()
211225
{
212-
return true;
226+
if (lock.owns())
227+
{
228+
lock.unlock();
229+
LeaveCritical();
230+
}
213231
}
214232

215-
~CCriticalBlock()
233+
bool TryEnter(const char* pszName, const char* pszFile, int nLine)
216234
{
217-
pcs->Leave();
235+
if (!lock.owns())
236+
{
237+
EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()));
238+
lock.try_lock();
239+
if (!lock.owns())
240+
LeaveCritical();
241+
}
242+
return lock.owns();
218243
}
219-
};
220-
221-
#define CRITICAL_BLOCK(cs) \
222-
if (CCriticalBlock criticalblock = CCriticalBlock(cs, #cs, __FILE__, __LINE__))
223-
224-
#define ENTER_CRITICAL_SECTION(cs) \
225-
(cs).Enter(#cs, __FILE__, __LINE__)
226-
227-
#define LEAVE_CRITICAL_SECTION(cs) \
228-
(cs).Leave()
229244

230-
/** RAII object that tries to acquire mutex. Needed for exception safety. */
231-
class CTryCriticalBlock
232-
{
233-
protected:
234-
CCriticalSection* pcs;
245+
CMutexLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) : lock(mutexIn, boost::interprocess::defer_lock)
246+
{
247+
if (fTry)
248+
TryEnter(pszName, pszFile, nLine);
249+
else
250+
Enter(pszName, pszFile, nLine);
251+
}
235252

236-
public:
237-
CTryCriticalBlock(CCriticalSection& csIn, const char* pszName, const char* pszFile, int nLine)
253+
~CMutexLock()
238254
{
239-
pcs = (csIn.TryEnter(pszName, pszFile, nLine) ? &csIn : NULL);
255+
if (lock.owns())
256+
LeaveCritical();
240257
}
241258

242-
operator bool() const
259+
operator bool()
243260
{
244-
return Entered();
261+
return lock.owns();
245262
}
246263

247-
~CTryCriticalBlock()
264+
boost::interprocess::scoped_lock<Mutex> &GetLock()
248265
{
249-
if (pcs)
250-
{
251-
pcs->Leave();
252-
}
266+
return lock;
253267
}
254-
bool Entered() const { return pcs != NULL; }
255268
};
256269

270+
typedef CMutexLock<CCriticalSection> CCriticalBlock;
271+
typedef CMutexLock<CWaitableCriticalSection> CWaitableCriticalBlock;
272+
typedef boost::interprocess::interprocess_condition CConditionVariable;
273+
274+
/** Wait for a given condition inside a WAITABLE_CRITICAL_BLOCK */
275+
#define WAIT(name,condition) \
276+
do { while(!(condition)) { (name).wait(waitablecriticalblock.GetLock()); } } while(0)
277+
278+
/** Notify waiting threads that a condition may hold now */
279+
#define NOTIFY(name) \
280+
do { (name).notify_one(); } while(0)
281+
282+
#define NOTIFY_ALL(name) \
283+
do { (name).notify_all(); } while(0)
284+
285+
#define CRITICAL_BLOCK(cs) \
286+
for (bool fcriticalblockonce=true; fcriticalblockonce; assert(("break caught by CRITICAL_BLOCK!" && !fcriticalblockonce)), fcriticalblockonce=false) \
287+
for (CCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__); fcriticalblockonce; fcriticalblockonce=false)
288+
289+
#define WAITABLE_CRITICAL_BLOCK(cs) \
290+
for (bool fcriticalblockonce=true; fcriticalblockonce; assert(("break caught by WAITABLE_CRITICAL_BLOCK!" && !fcriticalblockonce)), fcriticalblockonce=false) \
291+
for (CWaitableCriticalBlock waitablecriticalblock(cs, #cs, __FILE__, __LINE__); fcriticalblockonce; fcriticalblockonce=false)
292+
293+
#define ENTER_CRITICAL_SECTION(cs) \
294+
{ \
295+
EnterCritical(#cs, __FILE__, __LINE__, (void*)(&cs)); \
296+
(cs).lock(); \
297+
}
298+
299+
#define LEAVE_CRITICAL_SECTION(cs) \
300+
{ \
301+
(cs).unlock(); \
302+
LeaveCritical(); \
303+
}
304+
257305
#define TRY_CRITICAL_BLOCK(cs) \
258-
if (CTryCriticalBlock criticalblock = CTryCriticalBlock(cs, #cs, __FILE__, __LINE__))
306+
for (bool fcriticalblockonce=true; fcriticalblockonce; assert(("break caught by TRY_CRITICAL_BLOCK!" && !fcriticalblockonce)), fcriticalblockonce=false) \
307+
for (CCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__, true); fcriticalblockonce && (fcriticalblockonce = criticalblock); fcriticalblockonce=false)
308+
309+
310+
// This is exactly like std::string, but with a custom allocator.
311+
// (secure_allocator<> is defined in serialize.h)
312+
typedef std::basic_string<char, std::char_traits<char>, secure_allocator<char> > SecureString;
313+
259314

260315

261316

0 commit comments

Comments
 (0)