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

diff --git a/src/runtime/mgc0.c b/src/runtime/mgc0.c
index e369e5425c9ed0d1062cb7d98699f4e09012f66b..0de7b1bf4ad1f33ff48341b0beaa66d5ee7c658d 100644 (file)
--- 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 27a809a07eda41110a397246163dfad8d3e859aa..a84a32525eb2b5ea619a1f66d5d26e8b2740bfd6 100644 (file)
--- 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 8562b94076c4ba8b950c05402bd96fdd29849905..d1ea3ff73bf3fbe649013419c6dea02d4ae21187 100644 (file)
--- 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