Repository: trafficserver Updated Branches: refs/heads/master 5f332c4b9 -> ac4a7dbe2
TS-3402: rationalize lock debugging infrastructure Use SrcLoc to track lock holding locations. Enable lock diagnostics in DEBUG builds when the "lock" debug tag is set. Previously you had to do this *and* toggle some there magic #defines. Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/ac4a7dbe Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/ac4a7dbe Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/ac4a7dbe Branch: refs/heads/master Commit: ac4a7dbe26ec83b66d311d3b9e1f9280a99172f6 Parents: 5f332c4 Author: James Peach <[email protected]> Authored: Tue Jan 27 13:12:43 2015 -0800 Committer: James Peach <[email protected]> Committed: Mon Feb 23 12:20:20 2015 -0800 ---------------------------------------------------------------------- CHANGES | 2 ++ iocore/eventsystem/I_Lock.h | 72 +++++++++++++++++++--------------------- iocore/eventsystem/Lock.cc | 56 +++++++++++-------------------- lib/ts/Diags.h | 12 ++++++- 4 files changed, 67 insertions(+), 75 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac4a7dbe/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index da96344..eb1619f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ -*- coding: utf-8 -*- Changes with Apache Traffic Server 5.3.0 + *) [TS-3402] Rationalize lock debugging infrastructure. + *) [TS-3358] Add access checking to the management API. *) [TS-3400] Use common FNV hash code everywhere. http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac4a7dbe/iocore/eventsystem/I_Lock.h ---------------------------------------------------------------------- diff --git a/iocore/eventsystem/I_Lock.h b/iocore/eventsystem/I_Lock.h index d733e07..bc79f0b 100644 --- a/iocore/eventsystem/I_Lock.h +++ b/iocore/eventsystem/I_Lock.h @@ -49,7 +49,7 @@ */ # ifdef DEBUG -# define MUTEX_LOCK(_l,_m,_t) MutexLock _l(__FILE__,__LINE__,NULL,_m,_t) +# define MUTEX_LOCK(_l,_m,_t) MutexLock _l(DiagsMakeLocation(),NULL,_m,_t) # else # define MUTEX_LOCK(_l,_m,_t) MutexLock _l(_m,_t) # endif //DEBUG @@ -69,7 +69,7 @@ */ # define MUTEX_TRY_LOCK(_l,_m,_t) \ -MutexTryLock _l(__FILE__,__LINE__,(char*)NULL,_m,_t) +MutexTryLock _l(DiagsMakeLocation(),(char*)NULL,_m,_t) /** Attempts to acquire the lock to the ProxyMutex. @@ -86,7 +86,7 @@ MutexTryLock _l(__FILE__,__LINE__,(char*)NULL,_m,_t) */ # define MUTEX_TRY_LOCK_SPIN(_l,_m,_t,_sc) \ -MutexTryLock _l(__FILE__,__LINE__,(char*)NULL,_m,_t,_sc) +MutexTryLock _l(DiagsMakeLocation(),(char*)NULL,_m,_t,_sc) /** Attempts to acquire the lock to the ProxyMutex. @@ -104,7 +104,7 @@ MutexTryLock _l(__FILE__,__LINE__,(char*)NULL,_m,_t,_sc) */ # define MUTEX_TRY_LOCK_FOR(_l,_m,_t,_c) \ -MutexTryLock _l(__FILE__,__LINE__,NULL,_m,_t) +MutexTryLock _l(DiagsMakeLocation(),NULL,_m,_t) # else //DEBUG # define MUTEX_TRY_LOCK(_l,_m,_t) MutexTryLock _l(_m,_t) # define MUTEX_TRY_LOCK_SPIN(_l,_m,_t,_sc) MutexTryLock _l(_m,_t,_sc) @@ -129,11 +129,11 @@ MutexTryLock _l(__FILE__,__LINE__,NULL,_m,_t) // DEPRECATED DEPRECATED DEPRECATED #ifdef DEBUG # define MUTEX_TAKE_TRY_LOCK(_m,_t) \ -Mutex_trylock(__FILE__,__LINE__,(char*)NULL,_m,_t) +Mutex_trylock(DiagsMakeLocation(),(char*)NULL,_m,_t) # define MUTEX_TAKE_TRY_LOCK_FOR(_m,_t,_c) \ -Mutex_trylock(__FILE__,__LINE__,(char*)NULL,_m,_t) +Mutex_trylock(DiagsMakeLocation(),(char*)NULL,_m,_t) # define MUTEX_TAKE_TRY_LOCK_FOR_SPIN(_m,_t,_c,_sc) \ -Mutex_trylock_spin(__FILE__,__LINE__,NULL,_m,_t,_sc) +Mutex_trylock_spin(DiagsMakeLocation(),NULL,_m,_t,_sc) #else # define MUTEX_TAKE_TRY_LOCK(_m,_t) Mutex_trylock(_m,_t) # define MUTEX_TAKE_TRY_LOCK_FOR(_m,_t,_c) Mutex_trylock(_m,_t) @@ -143,9 +143,9 @@ Mutex_trylock_spin(_m,_t,_sc) #ifdef DEBUG # define MUTEX_TAKE_LOCK(_m,_t)\ -Mutex_lock(__FILE__,__LINE__,(char*)NULL,_m,_t) +Mutex_lock(DiagsMakeLocation(),(char*)NULL,_m,_t) # define MUTEX_TAKE_LOCK_FOR(_m,_t,_c) \ -Mutex_lock(__FILE__,__LINE__,NULL,_m,_t) +Mutex_lock(DiagsMakeLocation(),NULL,_m,_t) #else # define MUTEX_TAKE_LOCK(_m,_t) Mutex_lock(_m,_t) # define MUTEX_TAKE_LOCK_FOR(_m,_t,_c) Mutex_lock(_m,_t) @@ -159,9 +159,11 @@ class EThread; typedef EThread *EThreadPtr; typedef volatile EThreadPtr VolatileEThreadPtr; -inkcoreapi extern void lock_waiting(const char *file, int line, const char *handler); -inkcoreapi extern void lock_holding(const char *file, int line, const char *handler); -extern void lock_taken(const char *file, int line, const char *handler); +#if DEBUG +inkcoreapi extern void lock_waiting(const SrcLoc&, const char *handler); +inkcoreapi extern void lock_holding(const SrcLoc&, const char *handler); +inkcoreapi extern void lock_taken(const SrcLoc&, const char *handler); +#endif /** Lock object used in continuations and threads. @@ -215,8 +217,7 @@ public: #ifdef DEBUG ink_hrtime hold_time; - const char *file; - int line; + SrcLoc srcloc; const char *handler; # ifdef MAX_LOCK_TAKEN @@ -242,13 +243,14 @@ public: */ ProxyMutex() +#ifdef DEBUG + : srcloc(NULL, NULL, 0) +#endif { thread_holding = NULL; nthread_holding = 0; #ifdef DEBUG hold_time = 0; - file = NULL; - line = 0; handler = NULL; # ifdef MAX_LOCK_TAKEN taken = 0; @@ -285,7 +287,7 @@ extern inkcoreapi ClassAllocator<ProxyMutex> mutexAllocator; inline bool Mutex_trylock( #ifdef DEBUG - const char *afile, int aline, const char *ahandler, + const SrcLoc& location, const char *ahandler, #endif ProxyMutex * m, EThread * t) { @@ -295,7 +297,7 @@ Mutex_trylock( if (m->thread_holding != t) { if (!ink_mutex_try_acquire(&m->the_mutex)) { #ifdef DEBUG - lock_waiting(m->file, m->line, m->handler); + lock_waiting(m->srcloc, m->handler); #ifdef LOCK_CONTENTION_PROFILING m->unsuccessful_nonblocking_acquires++; m->nonblocking_acquires++; @@ -307,8 +309,7 @@ Mutex_trylock( } m->thread_holding = t; #ifdef DEBUG - m->file = afile; - m->line = aline; + m->srcloc = location; m->handler = ahandler; m->hold_time = ink_get_hrtime(); #ifdef MAX_LOCK_TAKEN @@ -331,7 +332,7 @@ Mutex_trylock( inline bool Mutex_trylock_spin( #ifdef DEBUG - const char *afile, int aline, const char *ahandler, + const SrcLoc& location, const char *ahandler, #endif ProxyMutex * m, EThread * t, int spincnt = 1) { @@ -345,7 +346,7 @@ Mutex_trylock_spin( } while (--spincnt); if (!locked) { #ifdef DEBUG - lock_waiting(m->file, m->line, m->handler); + lock_waiting(m->srcloc, m->handler); #ifdef LOCK_CONTENTION_PROFILING m->unsuccessful_nonblocking_acquires++; m->nonblocking_acquires++; @@ -358,8 +359,7 @@ Mutex_trylock_spin( m->thread_holding = t; ink_assert(m->thread_holding); #ifdef DEBUG - m->file = afile; - m->line = aline; + m->srcloc = location; m->handler = ahandler; m->hold_time = ink_get_hrtime(); #ifdef MAX_LOCK_TAKEN @@ -382,7 +382,7 @@ Mutex_trylock_spin( inline int Mutex_lock( #ifdef DEBUG - const char *afile, int aline, const char *ahandler, + const SrcLoc& location, const char *ahandler, #endif ProxyMutex * m, EThread * t) { @@ -393,8 +393,7 @@ Mutex_lock( m->thread_holding = t; ink_assert(m->thread_holding); #ifdef DEBUG - m->file = afile; - m->line = aline; + m->srcloc = location; m->handler = ahandler; m->hold_time = ink_get_hrtime(); #ifdef MAX_LOCK_TAKEN @@ -422,13 +421,12 @@ Mutex_unlock(ProxyMutex * m, EThread * t) if (!m->nthread_holding) { #ifdef DEBUG if (ink_get_hrtime() - m->hold_time > MAX_LOCK_TIME) - lock_holding(m->file, m->line, m->handler); + lock_holding(m->srcloc, m->handler); #ifdef MAX_LOCK_TAKEN if (m->taken > MAX_LOCK_TAKEN) - lock_taken(m->file, m->line, m->handler); + lock_taken(m->srcloc, m->handler); #endif //MAX_LOCK_TAKEN - m->file = NULL; - m->line = 0; + m->srcloc = SrcLoc(NULL, NULL, 0); m->handler = NULL; #endif //DEBUG ink_assert(m->thread_holding); @@ -448,13 +446,13 @@ private: public: MutexLock( #ifdef DEBUG - const char *afile, int aline, const char *ahandler, + const SrcLoc& location, const char *ahandler, #endif //DEBUG ProxyMutex * am, EThread * t):m(am) { Mutex_lock( #ifdef DEBUG - afile, aline, ahandler, + location, ahandler, #endif //DEBUG m, t); } @@ -476,26 +474,26 @@ private: public: MutexTryLock( #ifdef DEBUG - const char *afile, int aline, const char *ahandler, + const SrcLoc& location, const char *ahandler, #endif //DEBUG ProxyMutex * am, EThread * t) : m(am) { lock_acquired = Mutex_trylock( #ifdef DEBUG - afile, aline, ahandler, + location, ahandler, #endif //DEBUG m, t); } MutexTryLock( #ifdef DEBUG - const char *afile, int aline, const char *ahandler, + const SrcLoc& location, const char *ahandler, #endif //DEBUG ProxyMutex * am, EThread * t, int sp) : m(am) { lock_acquired = Mutex_trylock_spin( #ifdef DEBUG - afile, aline, ahandler, + location, ahandler, #endif //DEBUG m, t, sp); } http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac4a7dbe/iocore/eventsystem/Lock.cc ---------------------------------------------------------------------- diff --git a/iocore/eventsystem/Lock.cc b/iocore/eventsystem/Lock.cc index 8f9ee34..c58160a 100644 --- a/iocore/eventsystem/Lock.cc +++ b/iocore/eventsystem/Lock.cc @@ -29,56 +29,38 @@ **************************************************************************/ #include "P_EventSystem.h" - -ClassAllocator<ProxyMutex> mutexAllocator("mutexAllocator"); - -// #define ERROR_CONFIG_TAG_LOCKS - -#ifdef ERROR_CONFIG_TAG_LOCKS #include "Diags.h" -#endif -#ifdef DEBUG // debug build needs lock_* functions -#undef INK_NO_LOCKS -#endif +ClassAllocator<ProxyMutex> mutexAllocator("mutexAllocator"); void -lock_waiting(const char *file, int line, const char *handler) +lock_waiting(const SrcLoc& srcloc, const char *handler) { - (void) file; - (void) line; - (void) handler; -#ifdef ERROR_CONFIG_TAG_LOCKS - if (is_diags_on("locks")) - fprintf(stderr, "WARNING: waiting on lock %s:%d for %s\n", - file ? file : "UNKNOWN", line, handler ? handler : "UNKNOWN"); -#endif + if (is_diags_on("locks")) { + char buf[128]; + fprintf(stderr, "WARNING: waiting on lock %s for %s\n", + srcloc.str(buf, sizeof(buf)), handler ? handler : "UNKNOWN"); + } } void -lock_holding(const char *file, int line, const char *handler) +lock_holding(const SrcLoc& srcloc, const char *handler) { - (void) file; - (void) line; - (void) handler; -#ifdef ERROR_CONFIG_TAG_LOCKS - if (is_diags_on("locks")) - fprintf(stderr, "WARNING: holding lock %s:%d too long for %s\n", - file ? file : "UNKNOWN", line, handler ? handler : "UNKNOWN"); -#endif + if (is_diags_on("locks")) { + char buf[128]; + fprintf(stderr, "WARNING: holding lock %s too long for %s\n", + srcloc.str(buf, sizeof(buf)), handler ? handler : "UNKNOWN"); + } } void -lock_taken(const char *file, int line, const char *handler) +lock_taken(const SrcLoc& srcloc, const char *handler) { - (void) file; - (void) line; - (void) handler; -#ifdef ERROR_CONFIG_TAG_LOCKS - if (is_diags_on("locks")) - fprintf(stderr, "WARNING: lock %s:%d taken too many times for %s\n", - file ? file : "UNKNOWN", line, handler ? handler : "UNKNOWN"); -#endif + if (is_diags_on("locks")) { + char buf[128]; + fprintf(stderr, "WARNING: lock %s taken too many times for %s\n", + srcloc.str(buf, sizeof(buf)), handler ? handler : "UNKNOWN"); + } } #ifdef LOCK_CONTENTION_PROFILING http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac4a7dbe/lib/ts/Diags.h ---------------------------------------------------------------------- diff --git a/lib/ts/Diags.h b/lib/ts/Diags.h index bad36c5..7a228c5 100644 --- a/lib/ts/Diags.h +++ b/lib/ts/Diags.h @@ -107,16 +107,26 @@ class SrcLoc public: const char *file; const char *func; - const int line; + int line; bool valid() const { return file && line; } + SrcLoc(const SrcLoc& rhs) : file(rhs.file), func(rhs.func), line(rhs.line) { + } + SrcLoc(const char *_file, const char *_func, int _line) : file(_file), func(_func), line(_line) { } + SrcLoc& operator=(const SrcLoc& rhs) { + this->file = rhs.file; + this->func = rhs.func; + this->line = rhs.line; + return *this; + } + char * str(char *buf, int buflen) const; };
