From 91e8554b8b9edfb4b05b2c04a50daf4df8ffed7b Mon Sep 17 00:00:00 2001
From: Keith Randall <khr@golang.org>
Date: Wed, 8 Oct 2014 15:57:20 -0700
Subject: [PATCH] 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://golang.org/cl/158750043
---
 src/runtime/mgc0.c    |  2 ++
 src/runtime/runtime.h |  1 +
 src/runtime/stack.c   | 38 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/runtime/mgc0.c b/src/runtime/mgc0.c
index e369e5425c9..0de7b1bf4ad 100644
--- a/src/runtime/mgc0.c
+++ b/src/runtime/mgc0.c
@@ -1445,6 +1445,8 @@ gc(struct gc_args *args)
 	if(runtime·work.nproc > 1)
 		runtime·notesleep(&runtime·work.alldone);
 
+	runtime·shrinkfinish();
+
 	cachestats();
 	// next_gc calculation is tricky with concurrent sweep since we don't know size of live heap
 	// estimate what was live heap size after previous GC (for tracing only)
diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h
index 27a809a07ed..a84a32525eb 100644
--- a/src/runtime/runtime.h
+++ b/src/runtime/runtime.h
@@ -852,6 +852,7 @@ void	runtime·stackinit(void);
 Stack	runtime·stackalloc(uint32);
 void	runtime·stackfree(Stack);
 void	runtime·shrinkstack(G*);
+void	runtime·shrinkfinish(void);
 MCache*	runtime·allocmcache(void);
 void	runtime·freemcache(MCache*);
 void	runtime·mallocinit(void);
diff --git a/src/runtime/stack.c b/src/runtime/stack.c
index 8562b94076c..d1ea3ff73bf 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