[svn-commits] russell: branch 1.4 r164736 - in /branches/1.4: include/asterisk/ main/

[SVN commits to the Digium repositories]

Author: russell
Date: Tue Dec 16 11:06:29 2008
New Revision: 164736

URL: http://svn.digium.com/view/asterisk?view=rev&rev=164736
Log:
Fix memory leak and invalid reporting issues with DEBUG_THREADLOCALS.

One issue was that the ast_mutex_* API was being used within the context of the
thread local data destructors.  We would go off and allocate more thread local data
while the pthread lib was in the middle of destroying it all.  This led to a memory 
leak.

Another issue was an invalid argument being provided to the the object_add
API call.

(closes issue #13678)
Reported by: ys
Tested by: Russell

Modified:
    branches/1.4/include/asterisk/threadstorage.h
    branches/1.4/main/threadstorage.c

Modified: branches/1.4/include/asterisk/threadstorage.h
URL: http://svn.digium.com/view/asterisk/branches/1.4/include/asterisk/threadstorage.h?view=diff&rev=164736&r1=164735&r2=164736
==============================================================================
--- branches/1.4/include/asterisk/threadstorage.h (original)
+++ branches/1.4/include/asterisk/threadstorage.h Tue Dec 16 11:06:29 2008
@@ -159,7 +159,7 @@
 		if (!(buf = ast_calloc(1, init_size)))
 			return NULL;
 		pthread_setspecific(ts->key, buf);
-		__ast_threadstorage_object_add(&ts->key, init_size, file, function, line);
+		__ast_threadstorage_object_add(buf, init_size, file, function, line);
 	}
 
 	return buf;

Modified: branches/1.4/main/threadstorage.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/main/threadstorage.c?view=diff&rev=164736&r1=164735&r2=164736
==============================================================================
--- branches/1.4/main/threadstorage.c (original)
+++ branches/1.4/main/threadstorage.c Tue Dec 16 11:06:29 2008
@@ -51,8 +51,23 @@
 };
 
 static AST_LIST_HEAD_NOLOCK_STATIC(tls_objects, tls_object);
-AST_MUTEX_DEFINE_STATIC_NOTRACKING(threadstoragelock);
-
+
+/* Allow direct use of pthread_mutex_t and friends */
+#undef pthread_mutex_t
+#undef pthread_mutex_lock
+#undef pthread_mutex_unlock
+#undef pthread_mutex_init
+#undef pthread_mutex_destroy
+
+/*!
+ * \brief lock for the tls_objects list
+ *
+ * \note We can not use an ast_mutex_t for this.  The reason is that this
+ *       lock is used within the context of thread-local data destructors,
+ *       and the ast_mutex_* API uses thread-local data.  Allocating more
+ *       thread-local data at that point just causes a memory leak.
+ */
+static pthread_mutex_t threadstoragelock;
 
 void __ast_threadstorage_object_add(void *key, size_t len, const char *file, const char *function, unsigned int line)
 {
@@ -68,16 +83,16 @@
 	to->line = line;
 	to->thread = pthread_self();
 
-	ast_mutex_lock(&threadstoragelock);
+	pthread_mutex_lock(&threadstoragelock);
 	AST_LIST_INSERT_TAIL(&tls_objects, to, entry);
-	ast_mutex_unlock(&threadstoragelock);
+	pthread_mutex_unlock(&threadstoragelock);
 }
 
 void __ast_threadstorage_object_remove(void *key)
 {
 	struct tls_object *to;
 
-	ast_mutex_lock(&threadstoragelock);
+	pthread_mutex_lock(&threadstoragelock);
 	AST_LIST_TRAVERSE_SAFE_BEGIN(&tls_objects, to, entry) {
 		if (to->key == key) {
 			AST_LIST_REMOVE_CURRENT(&tls_objects, entry);
@@ -85,7 +100,7 @@
 		}
 	}
 	AST_LIST_TRAVERSE_SAFE_END;
-	ast_mutex_unlock(&threadstoragelock);
+	pthread_mutex_unlock(&threadstoragelock);
 	if (to)
 		free(to);
 }
@@ -94,7 +109,7 @@
 {
 	struct tls_object *to;
 
-	ast_mutex_lock(&threadstoragelock);
+	pthread_mutex_lock(&threadstoragelock);
 	AST_LIST_TRAVERSE_SAFE_BEGIN(&tls_objects, to, entry) {
 		if (to->key == key_old) {
 			to->key = key_new;
@@ -103,7 +118,7 @@
 		}
 	}
 	AST_LIST_TRAVERSE_SAFE_END;
-	ast_mutex_unlock(&threadstoragelock);
+	pthread_mutex_unlock(&threadstoragelock);
 }
 
 static int handle_show_allocations(int fd, int argc, char *argv[])
@@ -116,7 +131,7 @@
 	if (argc > 3)
 		fn = argv[3];
 
-	ast_mutex_lock(&threadstoragelock);
+	pthread_mutex_lock(&threadstoragelock);
 
 	AST_LIST_TRAVERSE(&tls_objects, to, entry) {
 		if (fn && strcasecmp(to->file, fn))
@@ -128,7 +143,7 @@
 		count++;
 	}
 
-	ast_mutex_unlock(&threadstoragelock);
+	pthread_mutex_unlock(&threadstoragelock);
 
 	ast_cli(fd, "%10d bytes allocated in %d allocation%s\n", (int) len, count, count > 1 ? "s" : "");
 	
@@ -152,7 +167,7 @@
 	if (argc > 3)
 		fn = argv[3];
 
-	ast_mutex_lock(&threadstoragelock);
+	pthread_mutex_lock(&threadstoragelock);
 
 	AST_LIST_TRAVERSE(&tls_objects, to, entry) {
 		if (fn && strcasecmp(to->file, fn))
@@ -174,7 +189,7 @@
 		file->count++;
 	}
 
-	ast_mutex_unlock(&threadstoragelock);
+	pthread_mutex_unlock(&threadstoragelock);
 	
 	AST_LIST_TRAVERSE(&file_summary, file, entry) {
 		len += file->len;
@@ -216,6 +231,7 @@
 
 void threadstorage_init(void)
 {
+	pthread_mutex_init(&threadstoragelock, NULL);
 	ast_cli_register_multiple(cli, sizeof(cli) / sizeof(cli[0]));
 }