From patchwork Fri Jan 21 23:33:54 2011
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: libgo patch committed: Avoid deadlock during gc
Date: Fri, 21 Jan 2011 13:33:54 -0000
From: Ian Taylor <iant@google.com>
X-Patchwork-Id: 79966
Message-Id: <mcrbp39de71.fsf@google.com>
To: gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com

This patch to libgo avoids a deadlock when the finalizer lock is held
while the garbage collector is running.  Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu.  Committed to mainline.

Ian


diff -r 8939fef83538 libgo/runtime/go-go.c
--- a/libgo/runtime/go-go.c	Fri Jan 21 13:59:14 2011 -0800
+++ b/libgo/runtime/go-go.c	Fri Jan 21 15:26:51 2011 -0800
@@ -315,6 +315,15 @@
       return;
     }
 
+  if (__sync_bool_compare_and_swap (&pm->holds_finlock, 1, 1))
+    {
+      /* Similarly, we can't interrupt the thread while it holds the
+	 finalizer lock.  Otherwise we can get into a deadlock when
+	 mark calls runtime_walkfintab.  */
+      __sync_bool_compare_and_swap (&pm->gcing_for_finlock, 0, 1);
+      return;
+    }
+
   stop_for_gc ();
 }
 
diff -r 8939fef83538 libgo/runtime/mfinal.c
--- a/libgo/runtime/mfinal.c	Fri Jan 21 13:59:14 2011 -0800
+++ b/libgo/runtime/mfinal.c	Fri Jan 21 15:26:51 2011 -0800
@@ -106,9 +106,13 @@
 		e->ft = ft;
 	}
 
+	if(!__sync_bool_compare_and_swap(&m->holds_finlock, 0, 1))
+		runtime_throw("finalizer deadlock");
+
 	runtime_lock(&finlock);
 	if(!runtime_mlookup(p, &base, nil, nil, &ref) || p != base) {
 		runtime_unlock(&finlock);
+		__sync_bool_compare_and_swap(&m->holds_finlock, 1, 0);
 		runtime_throw("addfinalizer on invalid pointer");
 	}
 	if(f == nil) {
@@ -116,12 +120,12 @@
 			lookfintab(&fintab, p, 1);
 			*ref &= ~RefHasFinalizer;
 		}
-		runtime_unlock(&finlock);
-		return;
+		goto unlock;
 	}
 
 	if(*ref & RefHasFinalizer) {
 		runtime_unlock(&finlock);
+		__sync_bool_compare_and_swap(&m->holds_finlock, 1, 0);
 		runtime_throw("double finalizer");
 	}
 	*ref |= RefHasFinalizer;
@@ -156,7 +160,14 @@
 	}
 
 	addfintab(&fintab, p, e);
+ unlock:
 	runtime_unlock(&finlock);
+
+	__sync_bool_compare_and_swap(&m->holds_finlock, 1, 0);
+
+	if(__sync_bool_compare_and_swap(&m->gcing_for_finlock, 1, 0)) {
+		__go_run_goroutine_gc(200);
+	}
 }
 
 // get finalizer; if del, delete finalizer.
@@ -166,9 +177,18 @@
 {
 	Finalizer *f;
 	
+	if(!__sync_bool_compare_and_swap(&m->holds_finlock, 0, 1))
+		runtime_throw("finalizer deadlock");
+
 	runtime_lock(&finlock);
 	f = lookfintab(&fintab, p, del);
 	runtime_unlock(&finlock);
+
+	__sync_bool_compare_and_swap(&m->holds_finlock, 1, 0);
+	if(__sync_bool_compare_and_swap(&m->gcing_for_finlock, 1, 0)) {
+		__go_run_goroutine_gc(201);
+	}
+
 	return f;
 }
 
@@ -178,6 +198,9 @@
 	void **key;
 	void **ekey;
 
+	if(!__sync_bool_compare_and_swap(&m->holds_finlock, 0, 1))
+		runtime_throw("finalizer deadlock");
+
 	scan((byte*)&fintab, sizeof fintab);
 	runtime_lock(&finlock);
 	key = fintab.key;
@@ -186,4 +209,9 @@
 		if(*key != nil && *key != ((void*)-1))
 			fn(*key);
 	runtime_unlock(&finlock);
+
+	__sync_bool_compare_and_swap(&m->holds_finlock, 1, 0);
+	if(__sync_bool_compare_and_swap(&m->gcing_for_finlock, 1, 0)) {
+		runtime_throw("walkfintab not called from gc");
+	}
 }
diff -r 8939fef83538 libgo/runtime/runtime.h
--- a/libgo/runtime/runtime.h	Fri Jan 21 13:59:14 2011 -0800
+++ b/libgo/runtime/runtime.h	Fri Jan 21 15:26:51 2011 -0800
@@ -97,6 +97,8 @@
 	int32	locks;
 	int32	nomemprof;
 	int32	gcing_for_prof;
+	int32	holds_finlock;
+	int32	gcing_for_finlock;
 	MCache	*mcache;
 
 	/* For the list of all threads.  */