From b039abfc3ec93debe732bb4824bebd098ab7a62a Mon Sep 17 00:00:00 2001
From: Dmitriy Vyukov <dvyukov@google.com>
Date: Tue, 21 Jan 2014 10:48:37 +0400
Subject: [PATCH] runtime: fix specials deadlock
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

The deadlock is between span->specialLock and proflock:

goroutine 11 [running]:
runtime.MProf_Free(0x7fa272d26508, 0xc210054180, 0xc0)
        src/pkg/runtime/mprof.goc:220 +0x27
runtime.freespecial(0x7fa272d1e088, 0xc210054180, 0xc0)
        src/pkg/runtime/mheap.c:691 +0x6a
runtime.freeallspecials(0x7fa272d1af50, 0xc210054180, 0xc0)
        src/pkg/runtime/mheap.c:717 +0xb5
runtime.free(0xc210054180)
        src/pkg/runtime/malloc.goc:190 +0xfd
selectgo(0x7fa272a5ef58)
        src/pkg/runtime/chan.c:1136 +0x2d8
runtime.selectgo(0xc210054180)
        src/pkg/runtime/chan.c:840 +0x12
runtime_test.func·058()
        src/pkg/runtime/proc_test.go:146 +0xb4
runtime.goexit()
        src/pkg/runtime/proc.c:1405
created by runtime_test.TestTimerFairness
        src/pkg/runtime/proc_test.go:152 +0xd1

goroutine 12 [running]:
addspecial(0xc2100540c0, 0x7fa272d1e0a0)
        src/pkg/runtime/mheap.c:569 +0x88
runtime.setprofilebucket(0xc2100540c0, 0x7fa272d26508)
        src/pkg/runtime/mheap.c:668 +0x73
runtime.MProf_Malloc(0xc2100540c0, 0xc0, 0x0)
        src/pkg/runtime/mprof.goc:212 +0x16b
runtime.mallocgc(0xc0, 0x0, 0xc200000000)
        src/pkg/runtime/malloc.goc:142 +0x239
runtime.mal(0xbc)
        src/pkg/runtime/malloc.goc:703 +0x38
newselect(0x2, 0x7fa272a5cf60)
        src/pkg/runtime/chan.c:632 +0x53
runtime.newselect(0xc200000002, 0xc21005f000)
        src/pkg/runtime/chan.c:615 +0x28
runtime_test.func·058()
        src/pkg/runtime/proc_test.go:146 +0x37
runtime.goexit()
        src/pkg/runtime/proc.c:1405
created by runtime_test.TestTimerFairness
        src/pkg/runtime/proc_test.go:152 +0xd1

Fixes #7099.

R=golang-codereviews, khr
CC=golang-codereviews
https://golang.org/cl/53120043
---
 src/pkg/runtime/mheap.c   | 16 +++++++++++++---
 src/pkg/runtime/mprof.goc |  7 ++++++-
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/pkg/runtime/mheap.c b/src/pkg/runtime/mheap.c
index c77772afbe..920d653682 100644
--- a/src/pkg/runtime/mheap.c
+++ b/src/pkg/runtime/mheap.c
@@ -703,9 +703,12 @@ runtime·freespecial(Special *s, void *p, uintptr size)
 void
 runtime·freeallspecials(MSpan *span, void *p, uintptr size)
 {
-	Special *s, **t;
+	Special *s, **t, *list;
 	uintptr offset;
 
+	// first, collect all specials into the list; then, free them
+	// this is required to not cause deadlock between span->specialLock and proflock
+	list = nil;
 	offset = (uintptr)p - (span->start << PageShift);
 	runtime·lock(&span->specialLock);
 	t = &span->specials;
@@ -714,10 +717,17 @@ runtime·freeallspecials(MSpan *span, void *p, uintptr size)
 			break;
 		if(offset == s->offset) {
 			*t = s->next;
-			if(!runtime·freespecial(s, p, size))
-				runtime·throw("can't explicitly free an object with a finalizer");
+			s->next = list;
+			list = s;
 		} else
 			t = &s->next;
 	}
 	runtime·unlock(&span->specialLock);
+
+	while(list != nil) {
+		s = list;
+		list = s->next;
+		if(!runtime·freespecial(s, p, size))
+			runtime·throw("can't explicitly free an object with a finalizer");
+	}
 }
diff --git a/src/pkg/runtime/mprof.goc b/src/pkg/runtime/mprof.goc
index 51d0224250..4cd92d04a1 100644
--- a/src/pkg/runtime/mprof.goc
+++ b/src/pkg/runtime/mprof.goc
@@ -209,8 +209,13 @@ runtime·MProf_Malloc(void *p, uintptr size, uintptr typ)
 	b = stkbucket(MProf, stk, nstk, true);
 	b->recent_allocs++;
 	b->recent_alloc_bytes += size;
-	runtime·setprofilebucket(p, b);
 	runtime·unlock(&proflock);
+
+	// Setprofilebucket locks a bunch of other mutexes, so we call it outside of proflock.
+	// This reduces potential contention and chances of deadlocks.
+	// Since the object must be alive during call to MProf_Malloc,
+	// it's fine to do this non-atomically.
+	runtime·setprofilebucket(p, b);
 }
 
 // Called when freeing a profiled block.
-- 
2.48.1