]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: fix specials deadlock
authorDmitriy Vyukov <dvyukov@google.com>
Tue, 21 Jan 2014 06:48:37 +0000 (10:48 +0400)
committerDmitriy Vyukov <dvyukov@google.com>
Tue, 21 Jan 2014 06:48:37 +0000 (10:48 +0400)
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
src/pkg/runtime/mprof.goc

index c77772afbe5c6e0d3dbd140ea8f9bf6171d65984..920d6536829f650cd660044fb24c2e58ef15f24a 100644 (file)
@@ -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");
+       }
 }
index 51d0224250f32838e6f7aa00827642cd9a2cf61c..4cd92d04a1219f8bde77d731254a5371c8005ac4 100644 (file)
@@ -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.