From 691a924baf30c73fd91a19b6b9679ab9da6e29b3 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 7 Mar 2011 21:56:40 +0000 Subject: [PATCH] Avoid race condition manipulating heap when goroutine exits. From-SVN: r170758 --- libgo/runtime/go-go.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/libgo/runtime/go-go.c b/libgo/runtime/go-go.c index 8c2de2877cf0..3d8e9e629084 100644 --- a/libgo/runtime/go-go.c +++ b/libgo/runtime/go-go.c @@ -92,18 +92,15 @@ remove_current_thread (void) if (list_entry->next != NULL) list_entry->next->prev = list_entry->prev; + /* This will look runtime_mheap as needed. */ runtime_MCache_ReleaseAll (mcache); - /* As soon as we release this look, a GC could run. Since this - thread is no longer on the list, the GC will not find our M - structure, so it could get freed at any time. That means that - any code from here to thread exit must not assume that the m is - valid. */ - m = NULL; - - i = pthread_mutex_unlock (&__go_thread_ids_lock); - __go_assert (i == 0); - + /* This should never deadlock--there shouldn't be any code that + holds the runtime_mheap lock when locking __go_thread_ids_lock. + We don't want to do this after releasing __go_thread_ids_lock + because it will mean that the garbage collector might run, and + the garbage collector does not try to lock runtime_mheap in all + cases since it knows it is running single-threaded. */ runtime_lock (&runtime_mheap); mstats.heap_alloc += mcache->local_alloc; mstats.heap_objects += mcache->local_objects; @@ -111,6 +108,16 @@ remove_current_thread (void) runtime_FixAlloc_Free (&runtime_mheap.cachealloc, mcache); runtime_unlock (&runtime_mheap); + /* As soon as we release this look, a GC could run. Since this + thread is no longer on the list, the GC will not find our M + structure, so it could get freed at any time. That means that + any code from here to thread exit must not assume that m is + valid. */ + m = NULL; + + i = pthread_mutex_unlock (&__go_thread_ids_lock); + __go_assert (i == 0); + free (list_entry); }