[asterisk-commits] dlee: branch dlee/ASTERISK-194630-startup-deadlock r398377 - in /team/dlee/AS...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Sep 5 11:04:17 CDT 2013


Author: dlee
Date: Thu Sep  5 11:04:15 2013
New Revision: 398377

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=398377
Log:
Who locks the lock trackers?

Modified:
    team/dlee/ASTERISK-194630-startup-deadlock/include/asterisk/lock.h
    team/dlee/ASTERISK-194630-startup-deadlock/main/lock.c

Modified: team/dlee/ASTERISK-194630-startup-deadlock/include/asterisk/lock.h
URL: http://svnview.digium.com/svn/asterisk/team/dlee/ASTERISK-194630-startup-deadlock/include/asterisk/lock.h?view=diff&rev=398377&r1=398376&r2=398377
==============================================================================
--- team/dlee/ASTERISK-194630-startup-deadlock/include/asterisk/lock.h (original)
+++ team/dlee/ASTERISK-194630-startup-deadlock/include/asterisk/lock.h Thu Sep  5 11:04:15 2013
@@ -432,39 +432,6 @@
 	}
 }
 
-static inline void ast_reentrancy_init(struct ast_lock_track **plt)
-{
-	pthread_mutexattr_t reentr_attr;
-	struct ast_lock_track *lt = *plt;
-
-	if (lt) {
-		fprintf(stderr, "%s: Double initialized lock tracking\n", __func__);
-#if defined(DO_CRASH) || defined(THREAD_CRASH)
-		abort();
-#else
-		return;
-#endif
-	}
-
-	lt = *plt = (struct ast_lock_track *) calloc(1, sizeof(*lt));
-
-	pthread_mutexattr_init(&reentr_attr);
-	pthread_mutexattr_settype(&reentr_attr, AST_MUTEX_KIND);
-	pthread_mutex_init(&lt->reentr_mutex, &reentr_attr);
-	pthread_mutexattr_destroy(&reentr_attr);
-}
-
-static inline void delete_reentrancy_cs(struct ast_lock_track **plt)
-{
-	struct ast_lock_track *lt;
-	if (*plt) {
-		lt = *plt;
-		pthread_mutex_destroy(&lt->reentr_mutex);
-		free(lt);
-		*plt = NULL;
-	}
-}
-
 #else /* !DEBUG_THREADS */
 
 #define	CHANNEL_DEADLOCK_AVOIDANCE(chan) \

Modified: team/dlee/ASTERISK-194630-startup-deadlock/main/lock.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/ASTERISK-194630-startup-deadlock/main/lock.c?view=diff&rev=398377&r1=398376&r2=398377
==============================================================================
--- team/dlee/ASTERISK-194630-startup-deadlock/main/lock.c (original)
+++ team/dlee/ASTERISK-194630-startup-deadlock/main/lock.c Thu Sep  5 11:04:15 2013
@@ -62,6 +62,65 @@
 }
 #endif	/* defined(DEBUG_THREADS) && defined(HAVE_BKTR) */
 
+#ifdef DEBUG_THREADS
+AST_MUTEX_DEFINE_STATIC(reentrancy_lock);
+
+static inline struct ast_lock_track *ast_get_reentrancy(struct ast_lock_track **plt)
+{
+	pthread_mutexattr_t reentr_attr;
+	struct ast_lock_track *lt;
+
+	/* It's a bit painful to lock a global mutex for every access to the
+	 * reentrancy structure, but it's necessary to ensure that we don't
+	 * double-allocate the structure or double-initialize the reentr_mutex.
+	 *
+	 * If you'd like to replace this with a double-checked lock, be sure to
+	 * properly volatile-ize everything to avoid optimizer bugs.
+	 *
+	 * We also have to use the underlying pthread calls for manipulating
+	 * the mutex, because this is called from the Asterisk mutex code.
+	 */
+	pthread_mutex_lock(&reentrancy_lock.mutex);
+
+	if (*plt) {
+		pthread_mutex_unlock(&reentrancy_lock.mutex);
+		return *plt;
+	}
+
+	lt = *plt = (struct ast_lock_track *) calloc(1, sizeof(*lt));
+
+	if (!lt) {
+		fprintf(stderr, "%s: Failed to allocate lock tracking\n", __func__);
+#if defined(DO_CRASH) || defined(THREAD_CRASH)
+		abort();
+#else
+		pthread_mutex_unlock(&reentrancy_lock.mutex);
+		return;
+#endif
+	}
+
+	pthread_mutexattr_init(&reentr_attr);
+	pthread_mutexattr_settype(&reentr_attr, AST_MUTEX_KIND);
+	pthread_mutex_init(&lt->reentr_mutex, &reentr_attr);
+	pthread_mutexattr_destroy(&reentr_attr);
+
+	pthread_mutex_unlock(&reentrancy_lock.mutex);
+	return lt;
+}
+
+static inline void delete_reentrancy_cs(struct ast_lock_track **plt)
+{
+	struct ast_lock_track *lt;
+	if (*plt) {
+		lt = *plt;
+		pthread_mutex_destroy(&lt->reentr_mutex);
+		free(lt);
+		*plt = NULL;
+	}
+}
+
+#endif /* DEBUG_THREADS */
+
 int __ast_pthread_mutex_init(int tracking, const char *filename, int lineno, const char *func,
 						const char *mutex_name, ast_mutex_t *t)
 {
@@ -84,7 +143,7 @@
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
 	if ((t->tracking = tracking)) {
-		ast_reentrancy_init(&t->track);
+		ast_get_reentrancy(&t->track);
 	}
 #endif /* DEBUG_THREADS */
 
@@ -102,7 +161,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 
 #if defined(AST_MUTEX_INIT_W_CONSTRUCTORS) && defined(CAN_COMPARE_MUTEX_TO_INIT_VALUE)
@@ -119,10 +178,9 @@
 	}
 #endif
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	res = pthread_mutex_trylock(&t->mutex);
 	switch (res) {
@@ -180,7 +238,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -201,10 +259,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
@@ -312,7 +369,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -333,10 +390,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
@@ -386,7 +442,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -405,10 +461,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 		ast_reentrancy_lock(lt);
@@ -496,7 +551,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -515,10 +570,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 		ast_reentrancy_lock(lt);
@@ -604,7 +658,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -623,10 +677,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 		ast_reentrancy_lock(lt);
@@ -722,7 +775,7 @@
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
 	if ((t->tracking = tracking)) {
-		ast_reentrancy_init(&t->track);
+		ast_get_reentrancy(&t->track);
 	}
 #endif /* DEBUG_THREADS */
 
@@ -785,7 +838,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -806,10 +859,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 		ast_reentrancy_lock(lt);
@@ -871,7 +923,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -892,10 +944,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
@@ -990,7 +1041,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -1011,10 +1062,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
@@ -1113,7 +1163,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -1134,10 +1184,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
@@ -1216,7 +1265,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -1237,10 +1286,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
@@ -1322,7 +1370,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
 #endif
@@ -1343,10 +1391,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
@@ -1392,7 +1439,7 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt;
+	struct ast_lock_track *lt = NULL;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
 #endif
@@ -1413,10 +1460,9 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking && !t->track) {
-		ast_reentrancy_init(&t->track);
-	}
-	lt = t->track;
+	if (t->tracking) {
+		lt = ast_get_reentrancy(&t->track);
+	}
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR




More information about the asterisk-commits mailing list