runtime: delay freeing of shrunk stacks until gc is done.

This change prevents confusion in the garbage collector.
The collector wants to make sure that every pointer it finds
isn't junk.  Its criteria for junk is (among others) points
to a "free" span.

Because the stack shrinker modifies pointers in the heap,
there is a race condition between the GC scanner and the
shrinker.  The GC scanner can see old pointers (pointers to
freed stacks).  In particular this happens with SudoG.elem
pointers.

Normally this is not a problem, as pointers into stack spans
are ok.  But if the freed stack is the last one in its span,
the span is marked as "free" instead of "contains stacks".

This change makes sure that even if the GC scanner sees
an old pointer, the span into which it points is still
marked as "contains stacks", and thus the GC doesn't
complain about it.

This change will make the GC pause a tiny bit slower, as
the stack freeing now happens in serial with the mark pause.
We could delay the freeing until the mutators start back up,
but this is the simplest change for now.

TBR=dvyukov
CC=golang-codereviews
https://codereview.appspot.com/158750043

1 files changed, 37 insertions, 1 deletions

diff --git a/src/runtime/stack.c b/src/runtime/stack.c
index 8562b9407..d1ea3ff73 100644
--- a/src/runtime/stack.c
+++ b/src/runtime/stack.c
@@ -36,6 +36,8 @@ MSpan runtime·stackpool[NumStackOrders];
 Mutex runtime·stackpoolmu;
 // TODO: one lock per order?
 
+static Stack stackfreequeue;
+
 void
 runtime·stackinit(void)
 {
@@ -656,7 +658,24 @@ copystack(G *gp, uintptr newsize)
 		while(p < ep)
 			*p++ = 0xfc;
 	}
-	runtime·stackfree(old);
+	if(newsize > old.hi-old.lo) {
+		// growing, free stack immediately
+		runtime·stackfree(old);
+	} else {
+		// shrinking, queue up free operation.  We can't actually free the stack
+		// just yet because we might run into the following situation:
+		// 1) GC starts, scans a SudoG but does not yet mark the SudoG.elem pointer
+		// 2) The stack that pointer points to is shrunk
+		// 3) The old stack is freed
+		// 4) The containing span is marked free
+		// 5) GC attempts to mark the SudoG.elem pointer.  The marking fails because
+		//    the pointer looks like a pointer into a free span.
+		// By not freeing, we prevent step #4 until GC is done.
+		runtime·lock(&runtime·stackpoolmu);
+		*(Stack*)old.lo = stackfreequeue;
+		stackfreequeue = old;
+		runtime·unlock(&runtime·stackpoolmu);
+	}
 }
 
 // round x up to a power of 2.
@@ -841,6 +860,23 @@ runtime·shrinkstack(G *gp)
 	copystack(gp, newsize);
 }
 
+// Do any delayed stack freeing that was queued up during GC.
+void
+runtime·shrinkfinish(void)
+{
+	Stack s, t;
+
+	runtime·lock(&runtime·stackpoolmu);
+	s = stackfreequeue;
+	stackfreequeue = (Stack){0,0};
+	runtime·unlock(&runtime·stackpoolmu);
+	while(s.lo != 0) {
+		t = *(Stack*)s.lo;
+		runtime·stackfree(s);
+		s = t;
+	}
+}
+
 static void badc(void);
 
 #pragma textflag NOSPLIT