[asterisk-commits] dlee: branch dlee/ASTERISK-22455-deadlock r398278 - /team/dlee/ASTERISK-22455...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed Sep 4 14:57:46 CDT 2013
Author: dlee
Date: Wed Sep 4 14:57:43 2013
New Revision: 398278
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=398278
Log:
Fix optional_api deadlocks.
* Don't capture backtraces while a lock is held.
* Reset reentrancy on cond_{timed,}wait
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=398278&r1=398277&r2=398278
==============================================================================
--- team/dlee/ASTERISK-22455-deadlock/main/lock.c (original)
+++ team/dlee/ASTERISK-22455-deadlock/main/lock.c Wed Sep 4 14:57:43 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(<->backtrace[lt->reentrancy]);
+ lt->backtrace[lt->reentrancy] = tmp;
bt = <->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(<->backtrace[lt->reentrancy]);
+ lt->backtrace[lt->reentrancy] = tmp;
bt = <->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 = <->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(<->backtrace[lt->reentrancy]);
- bt = <->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 = <->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(<->backtrace[lt->reentrancy]);
- bt = <->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(<->backtrace[lt->reentrancy]);
+ lt->backtrace[lt->reentrancy] = tmp;
bt = <->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(<->backtrace[lt->reentrancy]);
+ lt->backtrace[lt->reentrancy] = tmp;
bt = <->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(<->backtrace[lt->reentrancy]);
+ lt->backtrace[lt->reentrancy] = tmp;
bt = <->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(<->backtrace[lt->reentrancy]);
+ lt->backtrace[lt->reentrancy] = tmp;
bt = <->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(<->backtrace[lt->reentrancy]);
+ lt->backtrace[lt->reentrancy] = tmp;
bt = <->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(<->backtrace[lt->reentrancy]);
+ lt->backtrace[lt->reentrancy] = tmp;
bt = <->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