Patchwork libgo patch committed: Avoid deadlock during gc

login
register
mail settings
Submitter Ian Taylor
Date Jan. 21, 2011, 11:33 p.m.
Message ID <mcrbp39de71.fsf@google.com>
Download mbox | patch
Permalink /patch/79966/
State New
Headers show

Comments

Ian Taylor - Jan. 21, 2011, 11:33 p.m.
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

Patch

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.  */