[asterisk-commits] dlee: branch dlee/ASTERISK-22455-deadlock r398290 - /team/dlee/ASTERISK-22455...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Sep 4 16:54:38 CDT 2013


Author: dlee
Date: Wed Sep  4 16:54:36 2013
New Revision: 398290

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=398290
Log:
Don't obtain bactraces while a lock is held

Modified:
    team/dlee/ASTERISK-22455-deadlock/main/lock.c

Modified: team/dlee/ASTERISK-22455-deadlock/main/lock.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/ASTERISK-22455-deadlock/main/lock.c?view=diff&rev=398290&r1=398289&r2=398290
==============================================================================
--- team/dlee/ASTERISK-22455-deadlock/main/lock.c (original)
+++ team/dlee/ASTERISK-22455-deadlock/main/lock.c Wed Sep  4 16:54:36 2013
@@ -208,12 +208,20 @@
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
+		struct ast_bt tmp;
+
+		/* The implementation of backtrace() may have its own locks.
+		 * Capture the backtrace outside of the reentrancy lock to
+		 * avoid deadlocks. See ASTERISK-22455. */
+		ast_bt_get_addresses(&tmp);
+
 		ast_reentrancy_lock(lt);
 		if (lt->reentrancy != AST_MAX_REENTRANCY) {
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			lt->backtrace[lt->reentrancy] = tmp;
 			bt = &lt->backtrace[lt->reentrancy];
 		}
 		ast_reentrancy_unlock(lt);
+
 		ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, t, bt);
 #else
 		ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, t);
@@ -340,12 +348,20 @@
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
+		struct ast_bt tmp;
+
+		/* The implementation of backtrace() may have its own locks.
+		 * Capture the backtrace outside of the reentrancy lock to
+		 * avoid deadlocks. See ASTERISK-22455. */
+		ast_bt_get_addresses(&tmp);
+
 		ast_reentrancy_lock(lt);
 		if (lt->reentrancy != AST_MAX_REENTRANCY) {
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			lt->backtrace[lt->reentrancy] = tmp;
 			bt = &lt->backtrace[lt->reentrancy];
 		}
 		ast_reentrancy_unlock(lt);
+
 		ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, t, bt);
 #else
 		ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, t);
@@ -497,6 +513,7 @@
 
 #ifdef DEBUG_THREADS
 	struct ast_lock_track *lt;
+	struct ast_lock_track lt_orig;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -533,24 +550,11 @@
 			DO_THREAD_CRASH;
 		}
 
-		if (--lt->reentrancy < 0) {
-			__ast_mutex_logger("%s line %d (%s): mutex '%s' freed more times than we've locked!\n",
-				   filename, lineno, func, mutex_name);
-			lt->reentrancy = 0;
-		}
-
-		if (lt->reentrancy < AST_MAX_REENTRANCY) {
-			lt->file[lt->reentrancy] = NULL;
-			lt->lineno[lt->reentrancy] = 0;
-			lt->func[lt->reentrancy] = NULL;
-			lt->thread[lt->reentrancy] = 0;
-		}
-
-#ifdef HAVE_BKTR
-		if (lt->reentrancy) {
-			bt = &lt->backtrace[lt->reentrancy - 1];
-		}
-#endif
+		/* Waiting on a condition completely suspends a recursive mutex,
+		 * even if it's been recursively locked multiple times. Make a
+		 * copy of the lock tracking, and reset reentrancy to zero */
+		lt_orig = *lt;
+		lt->reentrancy = 0;
 		ast_reentrancy_unlock(lt);
 
 #ifdef HAVE_BKTR
@@ -569,21 +573,13 @@
 				   filename, lineno, func, strerror(res));
 		DO_THREAD_CRASH;
 	} else if (t->tracking) {
-		ast_reentrancy_lock(lt);
-		if (lt->reentrancy < AST_MAX_REENTRANCY) {
-			lt->file[lt->reentrancy] = filename;
-			lt->lineno[lt->reentrancy] = lineno;
-			lt->func[lt->reentrancy] = func;
-			lt->thread[lt->reentrancy] = pthread_self();
-#ifdef HAVE_BKTR
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
-			bt = &lt->backtrace[lt->reentrancy];
-#endif
-			lt->reentrancy++;
-		} else {
-			__ast_mutex_logger("%s line %d (%s): '%s' really deep reentrancy!\n",
-							   filename, lineno, func, mutex_name);
-		}
+		pthread_mutex_t reentr_mutex_orig;
+		ast_reentrancy_lock(lt);
+		/* Restore lock tracking to what it was prior to the wait */
+		reentr_mutex_orig = lt->reentr_mutex;
+		*lt = lt_orig;
+		/* un-trash the mutex we just copied over */
+		lt->reentr_mutex = reentr_mutex_orig;
 		ast_reentrancy_unlock(lt);
 
 #ifdef HAVE_BKTR
@@ -605,6 +601,7 @@
 
 #ifdef DEBUG_THREADS
 	struct ast_lock_track *lt;
+	struct ast_lock_track lt_orig;
 	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
@@ -641,23 +638,11 @@
 			DO_THREAD_CRASH;
 		}
 
-		if (--lt->reentrancy < 0) {
-			__ast_mutex_logger("%s line %d (%s): mutex '%s' freed more times than we've locked!\n",
-					   filename, lineno, func, mutex_name);
-			lt->reentrancy = 0;
-		}
-
-		if (lt->reentrancy < AST_MAX_REENTRANCY) {
-			lt->file[lt->reentrancy] = NULL;
-			lt->lineno[lt->reentrancy] = 0;
-			lt->func[lt->reentrancy] = NULL;
-			lt->thread[lt->reentrancy] = 0;
-		}
-#ifdef HAVE_BKTR
-		if (lt->reentrancy) {
-			bt = &lt->backtrace[lt->reentrancy - 1];
-		}
-#endif
+		/* Waiting on a condition completely suspends a recursive mutex,
+		 * even if it's been recursively locked multiple times. Make a
+		 * copy of the lock tracking, and reset reentrancy to zero */
+		lt_orig = *lt;
+		lt->reentrancy = 0;
 		ast_reentrancy_unlock(lt);
 
 #ifdef HAVE_BKTR
@@ -676,21 +661,13 @@
 				   filename, lineno, func, strerror(res));
 		DO_THREAD_CRASH;
 	} else if (t->tracking) {
-		ast_reentrancy_lock(lt);
-		if (lt->reentrancy < AST_MAX_REENTRANCY) {
-			lt->file[lt->reentrancy] = filename;
-			lt->lineno[lt->reentrancy] = lineno;
-			lt->func[lt->reentrancy] = func;
-			lt->thread[lt->reentrancy] = pthread_self();
-#ifdef HAVE_BKTR
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
-			bt = &lt->backtrace[lt->reentrancy];
-#endif
-			lt->reentrancy++;
-		} else {
-			__ast_mutex_logger("%s line %d (%s): '%s' really deep reentrancy!\n",
-							   filename, lineno, func, mutex_name);
-		}
+		pthread_mutex_t reentr_mutex_orig;
+		ast_reentrancy_lock(lt);
+		/* Restore lock tracking to what it was prior to the wait */
+		reentr_mutex_orig = lt->reentr_mutex;
+		*lt = lt_orig;
+		/* un-trash the mutex we just copied over */
+		lt->reentr_mutex = reentr_mutex_orig;
 		ast_reentrancy_unlock(lt);
 
 #ifdef HAVE_BKTR
@@ -899,12 +876,20 @@
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
+		struct ast_bt tmp;
+
+		/* The implementation of backtrace() may have its own locks.
+		 * Capture the backtrace outside of the reentrancy lock to
+		 * avoid deadlocks. See ASTERISK-22455. */
+		ast_bt_get_addresses(&tmp);
+
 		ast_reentrancy_lock(lt);
 		if (lt->reentrancy != AST_MAX_REENTRANCY) {
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			lt->backtrace[lt->reentrancy] = tmp;
 			bt = &lt->backtrace[lt->reentrancy];
 		}
 		ast_reentrancy_unlock(lt);
+
 		ast_store_lock_info(AST_RDLOCK, filename, line, func, name, t, bt);
 #else
 		ast_store_lock_info(AST_RDLOCK, filename, line, func, name, t);
@@ -1018,12 +1003,20 @@
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
+		struct ast_bt tmp;
+
+		/* The implementation of backtrace() may have its own locks.
+		 * Capture the backtrace outside of the reentrancy lock to
+		 * avoid deadlocks. See ASTERISK-22455. */
+		ast_bt_get_addresses(&tmp);
+
 		ast_reentrancy_lock(lt);
 		if (lt->reentrancy != AST_MAX_REENTRANCY) {
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			lt->backtrace[lt->reentrancy] = tmp;
 			bt = &lt->backtrace[lt->reentrancy];
 		}
 		ast_reentrancy_unlock(lt);
+
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t, bt);
 #else
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t);
@@ -1141,12 +1134,20 @@
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
+		struct ast_bt tmp;
+
+		/* The implementation of backtrace() may have its own locks.
+		 * Capture the backtrace outside of the reentrancy lock to
+		 * avoid deadlocks. See ASTERISK-22455. */
+		ast_bt_get_addresses(&tmp);
+
 		ast_reentrancy_lock(lt);
 		if (lt->reentrancy != AST_MAX_REENTRANCY) {
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			lt->backtrace[lt->reentrancy] = tmp;
 			bt = &lt->backtrace[lt->reentrancy];
 		}
 		ast_reentrancy_unlock(lt);
+
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t, bt);
 #else
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t);
@@ -1244,12 +1245,20 @@
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
+		struct ast_bt tmp;
+
+		/* The implementation of backtrace() may have its own locks.
+		 * Capture the backtrace outside of the reentrancy lock to
+		 * avoid deadlocks. See ASTERISK-22455. */
+		ast_bt_get_addresses(&tmp);
+
 		ast_reentrancy_lock(lt);
 		if (lt->reentrancy != AST_MAX_REENTRANCY) {
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			lt->backtrace[lt->reentrancy] = tmp;
 			bt = &lt->backtrace[lt->reentrancy];
 		}
 		ast_reentrancy_unlock(lt);
+
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t, bt);
 #else
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t);
@@ -1350,12 +1359,20 @@
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
+		struct ast_bt tmp;
+
+		/* The implementation of backtrace() may have its own locks.
+		 * Capture the backtrace outside of the reentrancy lock to
+		 * avoid deadlocks. See ASTERISK-22455. */
+		ast_bt_get_addresses(&tmp);
+
 		ast_reentrancy_lock(lt);
 		if (lt->reentrancy != AST_MAX_REENTRANCY) {
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			lt->backtrace[lt->reentrancy] = tmp;
 			bt = &lt->backtrace[lt->reentrancy];
 		}
 		ast_reentrancy_unlock(lt);
+
 		ast_store_lock_info(AST_RDLOCK, filename, line, func, name, t, bt);
 #else
 		ast_store_lock_info(AST_RDLOCK, filename, line, func, name, t);
@@ -1420,12 +1437,20 @@
 
 	if (t->tracking) {
 #ifdef HAVE_BKTR
+		struct ast_bt tmp;
+
+		/* The implementation of backtrace() may have its own locks.
+		 * Capture the backtrace outside of the reentrancy lock to
+		 * avoid deadlocks. See ASTERISK-22455. */
+		ast_bt_get_addresses(&tmp);
+
 		ast_reentrancy_lock(lt);
 		if (lt->reentrancy != AST_MAX_REENTRANCY) {
-			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			lt->backtrace[lt->reentrancy] = tmp;
 			bt = &lt->backtrace[lt->reentrancy];
 		}
 		ast_reentrancy_unlock(lt);
+
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t, bt);
 #else
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t);




More information about the asterisk-commits mailing list