[asterisk-commits] russell: trunk r85995 - in /trunk: ./ include/asterisk/lock.h

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Oct 16 17:21:46 CDT 2007


Author: russell
Date: Tue Oct 16 17:21:45 2007
New Revision: 85995

URL: http://svn.digium.com/view/asterisk?view=rev&rev=85995
Log:
Merged revisions 85994 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r85994 | russell | 2007-10-16 17:14:36 -0500 (Tue, 16 Oct 2007) | 16 lines

Some locking errors exposed the fact that the lock debugging code itself was
not thread safe.  How ironic!  Anyway, these changes ensure that the code that
is accessing the lock debugging data is thread-safe.  

Many thanks to Ivan for finding and fixing the core issue here, and also 
thanks to those that tested the patch and provided test results.

(closes issue #10571)
(closes issue #10886)
(closes issue #10875)
(might close some others, as well ...)

Patches: (from issue #10571)
      ivan_ast_1_4_12_rel_patch_lock.h.diff uploaded by Ivan (license 229)
       - a few small changes by me

........

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

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.4-merged' - no diff available.

Modified: trunk/include/asterisk/lock.h
URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/lock.h?view=diff&rev=85995&r1=85994&r2=85995
==============================================================================
--- trunk/include/asterisk/lock.h (original)
+++ trunk/include/asterisk/lock.h Tue Oct 16 17:21:45 2007
@@ -118,6 +118,7 @@
 	int reentrancy;
 	const char *func[AST_MAX_REENTRANCY];
 	pthread_t thread[AST_MAX_REENTRANCY];
+	pthread_mutex_t reentr_mutex;
 };
 
 typedef struct ast_mutex_info ast_mutex_t;
@@ -166,11 +167,45 @@
 	memset(&empty_mutex, 0, sizeof(empty_mutex));
 }
 
+static inline void reentrancy_lock_cs(ast_mutex_t *p_ast_mutex)
+{
+	pthread_mutex_lock(&p_ast_mutex->reentr_mutex);
+}
+
+static inline void reentrancy_unlock_cs(ast_mutex_t *p_ast_mutex)
+{
+	pthread_mutex_unlock(&p_ast_mutex->reentr_mutex);
+}
+
+static inline void init_reentrancy_cs(ast_mutex_t *p_ast_mutex)
+{
+	int i;
+	static pthread_mutexattr_t reentr_attr;
+
+	for (i = 0; i < AST_MAX_REENTRANCY; i++) {
+		p_ast_mutex->file[i] = NULL;
+		p_ast_mutex->lineno[i] = 0;
+		p_ast_mutex->func[i] = NULL;
+		p_ast_mutex->thread[i] = 0;
+	}
+
+	p_ast_mutex->reentrancy = 0;
+
+	pthread_mutexattr_init(&reentr_attr);
+	pthread_mutexattr_settype(&reentr_attr, AST_MUTEX_KIND);
+	pthread_mutex_init(&p_ast_mutex->reentr_mutex, &reentr_attr);
+	pthread_mutexattr_destroy(&reentr_attr);
+}
+
+static inline void delete_reentrancy_cs(ast_mutex_t * p_ast_mutex)
+{
+	pthread_mutex_destroy(&p_ast_mutex->reentr_mutex);
+}
+
 static inline int __ast_pthread_mutex_init_attr(int track, const char *filename, int lineno, const char *func,
 						const char *mutex_name, ast_mutex_t *t,
 						pthread_mutexattr_t *attr) 
 {
-	int i;
 #ifdef AST_MUTEX_INIT_W_CONSTRUCTORS
 	int canlog = strcmp(filename, "logger.c");
 
@@ -186,13 +221,7 @@
 	}
 #endif
 
-	for (i = 0; i < AST_MAX_REENTRANCY; i++) {
-		t->file[i] = NULL;
-		t->lineno[i] = 0;
-		t->func[i] = NULL;
-		t->thread[i]  = 0;
-	}
-	t->reentrancy = 0;
+	init_reentrancy_cs(t);
 	t->track = track;
 
 	return pthread_mutex_init(&t->mutex, attr);
@@ -237,8 +266,10 @@
 	case EBUSY:
 		__ast_mutex_logger("%s line %d (%s): Error: attempt to destroy locked mutex '%s'.\n",
 				   filename, lineno, func, mutex_name);
+		reentrancy_lock_cs(t);
 		__ast_mutex_logger("%s line %d (%s): Error: '%s' was locked here.\n",
-				   t->file[t->reentrancy-1], t->lineno[t->reentrancy-1], t->func[t->reentrancy-1], mutex_name);
+			    t->file[t->reentrancy-1], t->lineno[t->reentrancy-1], t->func[t->reentrancy-1], mutex_name);
+		reentrancy_unlock_cs(t);
 		break;
 	}
 
@@ -249,9 +280,14 @@
 	else
 		t->mutex = PTHREAD_MUTEX_INIT_VALUE;
 #endif
+	reentrancy_lock_cs(t);
 	t->file[0] = filename;
 	t->lineno[0] = lineno;
 	t->func[0] = func;
+	t->reentrancy=0;
+	t->thread[0]  = 0;
+	reentrancy_unlock_cs(t);
+	delete_reentrancy_cs(t);
 
 	return res;
 }
@@ -290,9 +326,11 @@
 				if ((current - seconds) && (!((current - seconds) % 5))) {
 					__ast_mutex_logger("%s line %d (%s): Deadlock? waited %d sec for mutex '%s'?\n",
 							   filename, lineno, func, (int)(current - seconds), mutex_name);
+					reentrancy_lock_cs(t);
 					__ast_mutex_logger("%s line %d (%s): '%s' was locked here.\n",
 							   t->file[t->reentrancy-1], t->lineno[t->reentrancy-1],
 							   t->func[t->reentrancy-1], mutex_name);
+					reentrancy_unlock_cs(t);
 				}
 				usleep(200);
 			}
@@ -309,8 +347,7 @@
 #endif /* DETECT_DEADLOCKS */
 
 	if (!res) {
-		if (t->track)
-			ast_mark_lock_acquired();
+		reentrancy_lock_cs(t);
 		if (t->reentrancy < AST_MAX_REENTRANCY) {
 			t->file[t->reentrancy] = filename;
 			t->lineno[t->reentrancy] = lineno;
@@ -321,6 +358,9 @@
 			__ast_mutex_logger("%s line %d (%s): '%s' really deep reentrancy!\n",
 							   filename, lineno, func, mutex_name);
 		}
+		reentrancy_unlock_cs(t);
+		if (t->track)
+			ast_mark_lock_acquired();
 	} else {
 		if (t->track)
 			ast_remove_lock_info(&t->mutex);
@@ -350,8 +390,7 @@
 		ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, &t->mutex);
 
 	if (!(res = pthread_mutex_trylock(&t->mutex))) {
-		if (t->track)
-			ast_mark_lock_acquired();
+		reentrancy_lock_cs(t);
 		if (t->reentrancy < AST_MAX_REENTRANCY) {
 			t->file[t->reentrancy] = filename;
 			t->lineno[t->reentrancy] = lineno;
@@ -362,6 +401,9 @@
 			__ast_mutex_logger("%s line %d (%s): '%s' really deep reentrancy!\n",
 					   filename, lineno, func, mutex_name);
 		}
+		reentrancy_unlock_cs(t);
+		if (t->track)
+			ast_mark_lock_acquired();
 	} else if (t->track) {
 			ast_remove_lock_info(&t->mutex);
 	}
@@ -382,6 +424,7 @@
 	}
 #endif
 
+	reentrancy_lock_cs(t);
 	if (t->reentrancy && (t->thread[t->reentrancy-1] != pthread_self())) {
 		__ast_mutex_logger("%s line %d (%s): attempted unlock mutex '%s' without owning it!\n",
 				   filename, lineno, func, mutex_name);
@@ -402,6 +445,7 @@
 		t->func[t->reentrancy] = NULL;
 		t->thread[t->reentrancy] = 0;
 	}
+	reentrancy_unlock_cs(t);
 
 	if (t->track)
 		ast_remove_lock_info(&t->mutex);
@@ -453,6 +497,7 @@
 	}
 #endif
 
+	reentrancy_lock_cs(t);
 	if (t->reentrancy && (t->thread[t->reentrancy-1] != pthread_self())) {
 		__ast_mutex_logger("%s line %d (%s): attempted unlock mutex '%s' without owning it!\n",
 				   filename, lineno, func, mutex_name);
@@ -473,6 +518,7 @@
 		t->func[t->reentrancy] = NULL;
 		t->thread[t->reentrancy] = 0;
 	}
+	reentrancy_unlock_cs(t);
 
 	if (t->track)
 		ast_remove_lock_info(&t->mutex);
@@ -482,9 +528,7 @@
 				   filename, lineno, func, strerror(res));
 		DO_THREAD_CRASH;
 	} else {
-		if (t->track)
-			ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, &t->mutex);
-
+		reentrancy_lock_cs(t);
 		if (t->reentrancy < AST_MAX_REENTRANCY) {
 			t->file[t->reentrancy] = filename;
 			t->lineno[t->reentrancy] = lineno;
@@ -495,6 +539,10 @@
 			__ast_mutex_logger("%s line %d (%s): '%s' really deep reentrancy!\n",
 							   filename, lineno, func, mutex_name);
 		}
+		reentrancy_unlock_cs(t);
+
+		if (t->track)
+			ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, &t->mutex);
 	}
 
 	return res;
@@ -514,6 +562,7 @@
 	}
 #endif
 
+	reentrancy_lock_cs(t);
 	if (t->reentrancy && (t->thread[t->reentrancy-1] != pthread_self())) {
 		__ast_mutex_logger("%s line %d (%s): attempted unlock mutex '%s' without owning it!\n",
 				   filename, lineno, func, mutex_name);
@@ -534,6 +583,7 @@
 		t->func[t->reentrancy] = NULL;
 		t->thread[t->reentrancy] = 0;
 	}
+	reentrancy_unlock_cs(t);
 
 	if (t->track)
 		ast_remove_lock_info(&t->mutex);
@@ -543,9 +593,7 @@
 				   filename, lineno, func, strerror(res));
 		DO_THREAD_CRASH;
 	} else {
-		if (t->track)
-			ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, &t->mutex);
-
+		reentrancy_lock_cs(t);
 		if (t->reentrancy < AST_MAX_REENTRANCY) {
 			t->file[t->reentrancy] = filename;
 			t->lineno[t->reentrancy] = lineno;
@@ -556,6 +604,10 @@
 			__ast_mutex_logger("%s line %d (%s): '%s' really deep reentrancy!\n",
 							   filename, lineno, func, mutex_name);
 		}
+		reentrancy_unlock_cs(t);
+
+		if (t->track)
+			ast_store_lock_info(AST_MUTEX, filename, lineno, func, mutex_name, &t->mutex);
 	}
 
 	return res;




More information about the asterisk-commits mailing list