[asterisk-commits] russell: branch 1.6.1 r184547 - in /branches/1.6.1: ./ include/asterisk/lock.h

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Mar 26 21:25:38 CDT 2009 

Author: russell
Date: Thu Mar 26 21:25:31 2009
New Revision: 184547

URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=184547
Log:
Merged revisions 184531 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/trunk

........
r184531 | russell | 2009-03-26 21:20:23 -0500 (Thu, 26 Mar 2009) | 20 lines

Fix some issues with rwlock corruption that caused deadlock like symptoms.

When dvossel and I were doing some load testing last week, we noticed that we
could make Asterisk trunk lock up instantly when we started generating a bunch
of calls.  The backtraces of locked threads were bizarre, and many were stuck
on an _unlock_ of an rwlock.

The changes are:

1) Fix a number of places where a backtrace would be loaded into an invalid
   index of the backtrace array.  It's an off by one error, which ends up
   writing over the rwlock itself.

2) Ensure that in the array of held locks, we NULL out an index once it is
   not being used so that it's not confusing when analyzing its contents.

3) Remove a bunch of logging referring to an rwlock operating being done
   with "deep reentrancy".  It is normal for _many_ threads to hold a
   read lock on an rwlock.

........

Modified:
    branches/1.6.1/   (props changed)
    branches/1.6.1/include/asterisk/lock.h

Propchange: branches/1.6.1/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.

Modified: branches/1.6.1/include/asterisk/lock.h
URL: http://svn.digium.com/svn-view/asterisk/branches/1.6.1/include/asterisk/lock.h?view=diff&rev=184547&r1=184546&r2=184547
==============================================================================
--- branches/1.6.1/include/asterisk/lock.h (original)
+++ branches/1.6.1/include/asterisk/lock.h Thu Mar 26 21:25:31 2009
@@ -500,8 +500,10 @@
 	if (t->tracking) {
 #ifdef HAVE_BKTR
 		ast_reentrancy_lock(lt);
-		ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
-		bt = &lt->backtrace[lt->reentrancy];
+		if (lt->reentrancy != AST_MAX_REENTRANCY) {
+			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			bt = &lt->backtrace[lt->reentrancy];
+		}
 		ast_reentrancy_unlock(lt);
 		ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, t, bt);
 #else
@@ -622,8 +624,10 @@
 	if (t->tracking) {
 #ifdef HAVE_BKTR
 		ast_reentrancy_lock(lt);
-		ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
-		bt = &lt->backtrace[lt->reentrancy];
+		if (lt->reentrancy != AST_MAX_REENTRANCY) {
+			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			bt = &lt->backtrace[lt->reentrancy];
+		}
 		ast_reentrancy_unlock(lt);
 		ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, t, bt);
 #else
@@ -1069,6 +1073,7 @@
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
 #endif
+	int lock_found = 0;
 
 
 #ifdef AST_MUTEX_INIT_W_CONSTRUCTORS
@@ -1086,44 +1091,35 @@
 
 	ast_reentrancy_lock(lt);
 	if (lt->reentrancy) {
-		int lock_found = 0;
 		int i;
 		pthread_t self = pthread_self();
-		for (i = lt->reentrancy-1; i >= 0; --i) {
+		for (i = lt->reentrancy - 1; i >= 0; --i) {
 			if (lt->thread[i] == self) {
 				lock_found = 1;
-				if (i != lt->reentrancy-1) {
-					lt->file[i] = lt->file[lt->reentrancy-1];
-					lt->lineno[i] = lt->lineno[lt->reentrancy-1];
-					lt->func[i] = lt->func[lt->reentrancy-1];
-					lt->thread[i] = lt->thread[lt->reentrancy-1];
+				if (i != lt->reentrancy - 1) {
+					lt->file[i] = lt->file[lt->reentrancy - 1];
+					lt->lineno[i] = lt->lineno[lt->reentrancy - 1];
+					lt->func[i] = lt->func[lt->reentrancy - 1];
+					lt->thread[i] = lt->thread[lt->reentrancy - 1];
 				}
+#ifdef HAVE_BKTR
+				bt = &lt->backtrace[i];
+#endif
+				lt->file[lt->reentrancy - 1] = NULL;
+				lt->lineno[lt->reentrancy - 1] = 0;
+				lt->func[lt->reentrancy - 1] = NULL;
+				lt->thread[lt->reentrancy - 1] = AST_PTHREADT_NULL;
 				break;
 			}
 		}
-		if (!lock_found) {
-			__ast_mutex_logger("%s line %d (%s): attempted unlock rwlock '%s' without owning it!\n",
-						filename, line, func, name);
-			__ast_mutex_logger("%s line %d (%s): '%s' was last locked here.\n",
-					lt->file[lt->reentrancy-1], lt->lineno[lt->reentrancy-1], lt->func[lt->reentrancy-1], name);
-#ifdef HAVE_BKTR
-			__dump_backtrace(&lt->backtrace[lt->reentrancy-1], canlog);
-#endif
-			DO_THREAD_CRASH;
-		}
-	}
-
-	if (--lt->reentrancy < 0) {
+	}
+
+	if (lock_found && --lt->reentrancy < 0) {
 		__ast_mutex_logger("%s line %d (%s): rwlock '%s' freed more times than we've locked!\n",
 				filename, line, func, name);
 		lt->reentrancy = 0;
 	}
 
-#ifdef HAVE_BKTR
-	if (lt->reentrancy) {
-		bt = &lt->backtrace[lt->reentrancy - 1];
-	}
-#endif
 	ast_reentrancy_unlock(lt);
 
 	if (t->tracking) {
@@ -1171,8 +1167,10 @@
 	if (t->tracking) {
 #ifdef HAVE_BKTR
 		ast_reentrancy_lock(lt);
-		ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
-		bt = &lt->backtrace[lt->reentrancy];
+		if (lt->reentrancy != AST_MAX_REENTRANCY) {
+			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			bt = &lt->backtrace[lt->reentrancy];
+		}
 		ast_reentrancy_unlock(lt);
 		ast_store_lock_info(AST_RDLOCK, filename, line, func, name, t, bt);
 #else
@@ -1220,9 +1218,6 @@
 			lt->func[lt->reentrancy] = func;
 			lt->thread[lt->reentrancy] = pthread_self();
 			lt->reentrancy++;
-		} else {
-			__ast_mutex_logger("%s line %d (%s): read lock '%s' really deep reentrancy!\n",
-					filename, line, func, name);
 		}
 		ast_reentrancy_unlock(lt);
 		if (t->tracking) {
@@ -1280,8 +1275,10 @@
 	if (t->tracking) {
 #ifdef HAVE_BKTR
 		ast_reentrancy_lock(lt);
-		ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
-		bt = &lt->backtrace[lt->reentrancy];
+		if (lt->reentrancy != AST_MAX_REENTRANCY) {
+			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			bt = &lt->backtrace[lt->reentrancy];
+		}
 		ast_reentrancy_unlock(lt);
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t, bt);
 #else
@@ -1328,9 +1325,6 @@
 			lt->func[lt->reentrancy] = func;
 			lt->thread[lt->reentrancy] = pthread_self();
 			lt->reentrancy++;
-		} else {
-			__ast_mutex_logger("%s line %d (%s): write lock '%s' really deep reentrancy!\n",
-					filename, line, func, name);
 		}
 		ast_reentrancy_unlock(lt);
 		if (t->tracking) {
@@ -1364,7 +1358,6 @@
 {
 	int res;
 	struct ast_lock_track *lt = &t->track;
-	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
 #endif
@@ -1388,8 +1381,10 @@
 	if (t->tracking) {
 #ifdef HAVE_BKTR
 		ast_reentrancy_lock(lt);
-		ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
-		bt = &lt->backtrace[lt->reentrancy];
+		if (lt->reentrancy != AST_MAX_REENTRANCY) {
+			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			bt = &lt->backtrace[lt->reentrancy];
+		}
 		ast_reentrancy_unlock(lt);
 		ast_store_lock_info(AST_RDLOCK, filename, line, func, name, t, bt);
 #else
@@ -1405,9 +1400,6 @@
 			lt->func[lt->reentrancy] = func;
 			lt->thread[lt->reentrancy] = pthread_self();
 			lt->reentrancy++;
-		} else {
-			__ast_mutex_logger("%s line %d (%s): read lock '%s' really deep reentrancy!\n",
-					filename, line, func, name);
 		}
 		ast_reentrancy_unlock(lt);
 		if (t->tracking) {
@@ -1424,7 +1416,6 @@
 {
 	int res;
 	struct ast_lock_track *lt= &t->track;
-	int canlog = strcmp(filename, "logger.c") & t->tracking;
 #ifdef HAVE_BKTR
 	struct ast_bt *bt = NULL;
 #endif
@@ -1448,8 +1439,10 @@
 	if (t->tracking) {
 #ifdef HAVE_BKTR
 		ast_reentrancy_lock(lt);
-		ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
-		bt = &lt->backtrace[lt->reentrancy];
+		if (lt->reentrancy != AST_MAX_REENTRANCY) {
+			ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+			bt = &lt->backtrace[lt->reentrancy];
+		}
 		ast_reentrancy_unlock(lt);
 		ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t, bt);
 #else
@@ -1465,9 +1458,6 @@
 			lt->func[lt->reentrancy] = func;
 			lt->thread[lt->reentrancy] = pthread_self();
 			lt->reentrancy++;
-		} else {
-			__ast_mutex_logger("%s line %d (%s): write lock '%s' really deep reentrancy!\n",
-					filename, line, func, name);
 		}
 		ast_reentrancy_unlock(lt);
 		if (t->tracking) {

More information about the asterisk-commits mailing list