Add a magic number to debug_locks to better catch lock-coding errors.

Original description by Dave Hart:

 [This patch contains] the addition of a signature field to debug_lock
 initialized by the alloc routine and verified by the rest, to catch
 invalid lock pointers sooner. That helped me track down a nasty
 problem I had before adding the time.h include to
 libevent-internal.h, where different .c files had different ideas of
 whether event_base had a tod_tv_cache member depending on which
 included time.h before libevent-internal.h.
This commit is contained in:
Dave Hart 2011-03-07 23:08:42 -05:00 committed by Nick Mathewson
parent 2a83ecc849
commit b4a29c0a0f

View File

@ -109,7 +109,10 @@ evthread_set_condition_callbacks(const struct evthread_condition_callbacks *cbs)
return 0; return 0;
} }
#define DEBUG_LOCK_SIG 0xdeb0b10c
struct debug_lock { struct debug_lock {
unsigned signature;
unsigned locktype; unsigned locktype;
unsigned long held_by; unsigned long held_by;
/* XXXX if we ever use read-write locks, we will need a separate /* XXXX if we ever use read-write locks, we will need a separate
@ -133,6 +136,7 @@ debug_lock_alloc(unsigned locktype)
} else { } else {
result->lock = NULL; result->lock = NULL;
} }
result->signature = DEBUG_LOCK_SIG;
result->locktype = locktype; result->locktype = locktype;
result->count = 0; result->count = 0;
result->held_by = 0; result->held_by = 0;
@ -145,6 +149,7 @@ debug_lock_free(void *lock_, unsigned locktype)
struct debug_lock *lock = lock_; struct debug_lock *lock = lock_;
EVUTIL_ASSERT(lock->count == 0); EVUTIL_ASSERT(lock->count == 0);
EVUTIL_ASSERT(locktype == lock->locktype); EVUTIL_ASSERT(locktype == lock->locktype);
EVUTIL_ASSERT(DEBUG_LOCK_SIG == lock->signature);
if (_original_lock_fns.free) { if (_original_lock_fns.free) {
_original_lock_fns.free(lock->lock, _original_lock_fns.free(lock->lock,
lock->locktype|EVTHREAD_LOCKTYPE_RECURSIVE); lock->locktype|EVTHREAD_LOCKTYPE_RECURSIVE);
@ -157,6 +162,7 @@ debug_lock_free(void *lock_, unsigned locktype)
static void static void
evthread_debug_lock_mark_locked(unsigned mode, struct debug_lock *lock) evthread_debug_lock_mark_locked(unsigned mode, struct debug_lock *lock)
{ {
EVUTIL_ASSERT(DEBUG_LOCK_SIG == lock->signature);
++lock->count; ++lock->count;
if (!(lock->locktype & EVTHREAD_LOCKTYPE_RECURSIVE)) if (!(lock->locktype & EVTHREAD_LOCKTYPE_RECURSIVE))
EVUTIL_ASSERT(lock->count == 1); EVUTIL_ASSERT(lock->count == 1);
@ -189,12 +195,15 @@ debug_lock_lock(unsigned mode, void *lock_)
static void static void
evthread_debug_lock_mark_unlocked(unsigned mode, struct debug_lock *lock) evthread_debug_lock_mark_unlocked(unsigned mode, struct debug_lock *lock)
{ {
EVUTIL_ASSERT(DEBUG_LOCK_SIG == lock->signature);
if (lock->locktype & EVTHREAD_LOCKTYPE_READWRITE) if (lock->locktype & EVTHREAD_LOCKTYPE_READWRITE)
EVUTIL_ASSERT(mode & (EVTHREAD_READ|EVTHREAD_WRITE)); EVUTIL_ASSERT(mode & (EVTHREAD_READ|EVTHREAD_WRITE));
else else
EVUTIL_ASSERT((mode & (EVTHREAD_READ|EVTHREAD_WRITE)) == 0); EVUTIL_ASSERT((mode & (EVTHREAD_READ|EVTHREAD_WRITE)) == 0);
if (_evthread_id_fn) { if (_evthread_id_fn) {
EVUTIL_ASSERT(lock->held_by == _evthread_id_fn()); unsigned long me;
me = _evthread_id_fn();
EVUTIL_ASSERT(lock->held_by == me);
if (lock->count == 1) if (lock->count == 1)
lock->held_by = 0; lock->held_by = 0;
} }
@ -218,6 +227,7 @@ debug_cond_wait(void *_cond, void *_lock, const struct timeval *tv)
{ {
int r; int r;
struct debug_lock *lock = _lock; struct debug_lock *lock = _lock;
EVUTIL_ASSERT(DEBUG_LOCK_SIG == lock->signature);
EVLOCK_ASSERT_LOCKED(_lock); EVLOCK_ASSERT_LOCKED(_lock);
evthread_debug_lock_mark_unlocked(0, lock); evthread_debug_lock_mark_unlocked(0, lock);
r = _original_cond_fns.wait_condition(_cond, lock->lock, tv); r = _original_cond_fns.wait_condition(_cond, lock->lock, tv);