diff mbox

libgo patch committed: Fix locking for release and allocate cache

Message ID mcr7hfd0w9g.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Dec. 14, 2010, 5:16 a.m. UTC 
In the last libgo update I introduced a bug.  I changed the code to not
hold the thread_ids lock while releasing the cache.  That was a mistake,
because a garbage collection could occur after the thread_ids lock was
released but before the cache was released.  That could cause the
thread's m structure to be released, which could in turn cause a null
pointer dereference when updating statistics.

Conversely, it is no longer necessary to hold the thread_ids lock while
allocating the cache.  Previously that served as a lock for
runtime_mheap.cachealloc, but that is no longer necessary as the code
now uses the runtime_mheap lock instead.

This patch implements both changes.  Committed to mainline.

Ian
diff mbox

Patch

diff -r 3ac32bd87c72 libgo/runtime/go-go.c
--- a/libgo/runtime/go-go.c	Thu Dec 09 15:54:35 2010 -0800
+++ b/libgo/runtime/go-go.c	Mon Dec 13 21:11:03 2010 -0800
@@ -107,11 +107,11 @@ 
   if (list_entry->next != NULL)
     list_entry->next->prev = list_entry->prev;
 
+  runtime_MCache_ReleaseAll (mcache);
+
   i = pthread_mutex_unlock (&__go_thread_ids_lock);
   __go_assert (i == 0);
 
-  runtime_MCache_ReleaseAll (mcache);
-
   runtime_lock (&runtime_mheap);
   mstats.heap_alloc += mcache->local_alloc;
   mstats.heap_objects += mcache->local_objects;
@@ -225,14 +225,13 @@ 
 
   newm->list_entry = list_entry;
 
+  newm->mcache = runtime_allocmcache ();
+
   /* Add the thread to the list of all threads, marked as tentative
      since it is not yet ready to go.  */
   i = pthread_mutex_lock (&__go_thread_ids_lock);
   __go_assert (i == 0);
 
-  /* We use __go_thread_ids_lock as a lock for mheap.cachealloc.  */
-  newm->mcache = runtime_allocmcache ();
-
   if (__go_all_thread_ids != NULL)
     __go_all_thread_ids->prev = list_entry;
   list_entry->next = __go_all_thread_ids;