From 602762b2c03110a436f1d7714b278f615b81d592 Mon Sep 17 00:00:00 2001 From: Ken Raeburn Date: Fri, 14 Jan 2005 03:19:39 +0000 Subject: [PATCH] More paranoid checking... * k5-thread.h (k5_os_mutex) [pthread case]: Add new field "owner" if DEBUG_THREADS. (k5_pthread_mutex_lock, k5_pthread_mutex_unlock, k5_pthread_assert_locked): New macros/functions; if DEBUG_THREADS, set or check the owner field. (K5_OS_MUTEX_PARTIAL_INITIALIZER) [pthread case && DEBUG_THREADS]: Set the owner field. If PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP is defined, use it. (k5_os_mutex_lock, k5_os_mutex_unlock, k5_os_mutex_assert_locked) [pthread case]: Use k5_pthread_ versions. (k5_mutex_destroy): Update the location data with the mutex locked, before destroying it. (k5_mutex_unlock): Update the location data while the mutex is still locked, and check the assertion that the mutex really is locked. Convert inline function to macro. * k5-thread.h (krb5int_mutex_lock_update_stats, krb5int_mutex_unlock_update_stats, krb5int_mutex_report_stats) [!DEBUG_THREADS_STATS]: Declare KRB5_CALLCONV. ticket: 2878 status: open git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@17031 dc483132-0cff-0310-8789-dd5450dbe970 --- src/include/ChangeLog | 22 +++++++++ src/include/k5-thread.h | 100 +++++++++++++++++++++++++++++----------- 2 files changed, 96 insertions(+), 26 deletions(-) diff --git a/src/include/ChangeLog b/src/include/ChangeLog index 3249c35d0..95d117b8c 100644 --- a/src/include/ChangeLog +++ b/src/include/ChangeLog @@ -1,3 +1,25 @@ +2005-01-13 Ken Raeburn + + * k5-thread.h (k5_os_mutex) [pthread case]: Add new field "owner" + if DEBUG_THREADS. + (k5_pthread_mutex_lock, k5_pthread_mutex_unlock, + k5_pthread_assert_locked): New macros/functions; if DEBUG_THREADS, + set or check the owner field. + (K5_OS_MUTEX_PARTIAL_INITIALIZER) [pthread case && DEBUG_THREADS]: + Set the owner field. If PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP + is defined, use it. + (k5_os_mutex_lock, k5_os_mutex_unlock, k5_os_mutex_assert_locked) + [pthread case]: Use k5_pthread_ versions. + (k5_mutex_destroy): Update the location data with the mutex + locked, before destroying it. + (k5_mutex_unlock): Update the location data while the mutex is + still locked, and check the assertion that the mutex really is + locked. Convert inline function to macro. + + * k5-thread.h (krb5int_mutex_lock_update_stats, + krb5int_mutex_unlock_update_stats, krb5int_mutex_report_stats) + [!DEBUG_THREADS_STATS]: Declare KRB5_CALLCONV. + 2005-01-09 Ken Raeburn * k5-int.h (krb5int_zap_data): Fix preprocessor test for Windows. diff --git a/src/include/k5-thread.h b/src/include/k5-thread.h index 2d8c03413..32b793d37 100644 --- a/src/include/k5-thread.h +++ b/src/include/k5-thread.h @@ -232,12 +232,12 @@ typedef struct { #define K5_MUTEX_STATS_INIT { 0, {0}, {0}, {0}, {0} } typedef k5_debug_time_t k5_mutex_stats_tmp; #define k5_mutex_stats_start() get_current_time() -extern void krb5int_mutex_lock_update_stats(k5_debug_mutex_stats *m, - k5_mutex_stats_tmp startwait); -extern void krb5int_mutex_unlock_update_stats(k5_debug_mutex_stats *m); +void KRB5_CALLCONV krb5int_mutex_lock_update_stats(k5_debug_mutex_stats *m, + k5_mutex_stats_tmp start); +void KRB5_CALLCONV krb5int_mutex_unlock_update_stats(k5_debug_mutex_stats *m); #define k5_mutex_lock_update_stats krb5int_mutex_lock_update_stats #define k5_mutex_unlock_update_stats krb5int_mutex_unlock_update_stats -extern void krb5int_mutex_report_stats(); +void KRB5_CALLCONV krb5int_mutex_report_stats(/* k5_mutex_t *m */); #else @@ -253,7 +253,7 @@ typedef int k5_mutex_stats_tmp; /* If statistics tracking isn't enabled, these functions don't actually do anything. Declare anyways so we can do type checking etc. */ void KRB5_CALLCONV krb5int_mutex_lock_update_stats(k5_debug_mutex_stats *m, - k5_mutex_stats_tmp start); + k5_mutex_stats_tmp start); void KRB5_CALLCONV krb5int_mutex_unlock_update_stats(k5_debug_mutex_stats *m); void KRB5_CALLCONV krb5int_mutex_report_stats(/* k5_mutex_t *m */); @@ -461,17 +461,51 @@ typedef pthread_once_t k5_once_t; typedef struct { pthread_mutex_t p; +#ifdef DEBUG_THREADS + pthread_t owner; +#endif #ifdef USE_PTHREAD_LOCK_ONLY_IF_LOADED k5_os_nothread_mutex n; #endif } k5_os_mutex; +#ifdef DEBUG_THREADS +# ifdef __GNUC__ +# define k5_pthread_mutex_lock(M) \ + ({ \ + k5_os_mutex *_m2 = (M); \ + int _r2 = pthread_mutex_lock(&_m2->p); \ + if (_r2 == 0) _m2->owner = pthread_self(); \ + _r2; \ + }) +# else +static inline int +k5_pthread_mutex_lock(k5_os_mutex *m) +{ + int r = pthread_mutex_lock(&m->p); + if (r) + return r; + m->owner = pthread_self(); + return 0; +} +# endif +# define k5_pthread_assert_locked(M) \ + (assert(pthread_equal((M)->owner, pthread_self()))) +# define k5_pthread_mutex_unlock(M) \ + (assert(pthread_equal((M)->owner, pthread_self())), \ + (M)->owner = (pthread_t) 0, \ + pthread_mutex_unlock(&(M)->p)) +#else +# define k5_pthread_mutex_lock(M) pthread_mutex_lock(&(M)->p) +static inline void k5_pthread_assert_locked(pthread_mutex_t *m) { } +# define k5_pthread_mutex_unlock(M) pthread_mutex_unlock(&(M)->p) +#endif + /* Define as functions to: (1) eliminate "statement with no effect" warnings for "0" (2) encourage type-checking in calling code */ static inline void k5_pthread_assert_unlocked(pthread_mutex_t *m) { } -static inline void k5_pthread_assert_locked(pthread_mutex_t *m) { } #if defined(DEBUG_THREADS_SLOW) && HAVE_SCHED_H && (HAVE_SCHED_YIELD || HAVE_PRAGMA_WEAK_REF) # include @@ -519,8 +553,18 @@ static inline int return_after_yield(int r) #ifdef USE_PTHREAD_LOCK_ONLY_IF_LOADED -# define K5_OS_MUTEX_PARTIAL_INITIALIZER \ +# if defined(PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP) && defined(DEBUG_THREADS) +# define K5_OS_MUTEX_PARTIAL_INITIALIZER \ + { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, (pthread_t) 0, \ + K5_OS_NOTHREAD_MUTEX_PARTIAL_INITIALIZER } +# elif defined(DEBUG_THREADS) +# define K5_OS_MUTEX_PARTIAL_INITIALIZER \ + { PTHREAD_MUTEX_INITIALIZER, (pthread_t) 0, \ + K5_OS_NOTHREAD_MUTEX_PARTIAL_INITIALIZER } +# else +# define K5_OS_MUTEX_PARTIAL_INITIALIZER \ { PTHREAD_MUTEX_INITIALIZER, K5_OS_NOTHREAD_MUTEX_PARTIAL_INITIALIZER } +# endif # define k5_os_mutex_finish_init(M) \ k5_os_nothread_mutex_finish_init(&(M)->n) @@ -537,12 +581,12 @@ static inline int return_after_yield(int r) # define k5_os_mutex_lock(M) \ return_after_yield(K5_PTHREADS_LOADED \ - ? pthread_mutex_lock(&(M)->p) \ + ? k5_pthread_mutex_lock(&(M)->p) \ : k5_os_nothread_mutex_lock(&(M)->n)) # define k5_os_mutex_unlock(M) \ (MAYBE_SCHED_YIELD(), \ (K5_PTHREADS_LOADED \ - ? pthread_mutex_unlock(&(M)->p) \ + ? k5_pthread_mutex_unlock(M) \ : k5_os_nothread_mutex_unlock(&(M)->n))) # define k5_os_mutex_assert_unlocked(M) \ @@ -551,22 +595,32 @@ static inline int return_after_yield(int r) : k5_os_nothread_mutex_assert_unlocked(&(M)->n)) # define k5_os_mutex_assert_locked(M) \ (K5_PTHREADS_LOADED \ - ? k5_pthread_assert_locked(&(M)->p) \ + ? k5_pthread_assert_locked(M) \ : k5_os_nothread_mutex_assert_locked(&(M)->n)) #else -# define K5_OS_MUTEX_PARTIAL_INITIALIZER \ +# ifdef DEBUG_THREADS +# ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP +# define K5_OS_MUTEX_PARTIAL_INITIALIZER \ + { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, (pthread_t) 0 } +# else +# define K5_OS_MUTEX_PARTIAL_INITIALIZER \ + { PTHREAD_MUTEX_INITIALIZER, (pthread_t) 0 } +# endif +# else +# define K5_OS_MUTEX_PARTIAL_INITIALIZER \ { PTHREAD_MUTEX_INITIALIZER } +# endif static inline int k5_os_mutex_finish_init(k5_os_mutex *m) { return 0; } # define k5_os_mutex_init(M) pthread_mutex_init(&(M)->p, 0) # define k5_os_mutex_destroy(M) pthread_mutex_destroy(&(M)->p) -# define k5_os_mutex_lock(M) return_after_yield(pthread_mutex_lock(&(M)->p)) -# define k5_os_mutex_unlock(M) (MAYBE_SCHED_YIELD(),pthread_mutex_unlock(&(M)->p)) +# define k5_os_mutex_lock(M) return_after_yield(k5_pthread_mutex_lock(M)) +# define k5_os_mutex_unlock(M) (MAYBE_SCHED_YIELD(),k5_pthread_mutex_unlock(M)) # define k5_os_mutex_assert_unlocked(M) k5_pthread_assert_unlocked(&(M)->p) -# define k5_os_mutex_assert_locked(M) k5_pthread_assert_locked(&(M)->p) +# define k5_os_mutex_assert_locked(M) k5_pthread_assert_locked(M) #endif /* is pthreads always available? */ @@ -653,7 +707,7 @@ static inline int k5_mutex_finish_init_1(k5_mutex_t *m, k5_debug_loc l) #define k5_mutex_destroy(M) \ (k5_os_mutex_assert_unlocked(&(M)->os), \ krb5int_mutex_report_stats(M), \ - (M)->loc_last = K5_DEBUG_LOC, \ + k5_mutex_lock(M), (M)->loc_last = K5_DEBUG_LOC, k5_mutex_unlock(M), \ k5_os_mutex_destroy(&(M)->os)) #ifdef __GNUC__ #define k5_mutex_lock(M) \ @@ -680,17 +734,11 @@ static inline int k5_mutex_lock_1(k5_mutex_t *m, k5_debug_loc l) } #define k5_mutex_lock(M) k5_mutex_lock_1(M, K5_DEBUG_LOC) #endif -static inline int k5_mutex_unlock_1(k5_mutex_t *m, k5_debug_loc l) -{ - int err = 0; - k5_mutex_unlock_update_stats(&m->stats); - err = k5_os_mutex_unlock(&m->os); - if (err) - return err; - m->loc_last = l; - return err; -} -#define k5_mutex_unlock(M) k5_mutex_unlock_1(M, K5_DEBUG_LOC) +#define k5_mutex_unlock(M) \ + (k5_mutex_assert_locked(M), \ + k5_mutex_unlock_update_stats(&(M)->stats), \ + (M)->loc_last = K5_DEBUG_LOC, \ + k5_os_mutex_unlock(&(M)->os)) #define k5_mutex_assert_locked(M) k5_os_mutex_assert_locked(&(M)->os) #define k5_mutex_assert_unlocked(M) k5_os_mutex_assert_unlocked(&(M)->os) -- 2.26.2