Skip to content

Commit

Permalink
runtime/pprof: prevent a deadlock that SIGPROF might create on mips{,le}
Browse files Browse the repository at this point in the history
64bit atomics on mips/mipsle are implemented using spinlocks. If SIGPROF
is received while the program is in the critical section, it will try to
write the sample using the same spinlock, creating a deadloop.
Prevent it by creating a counter of SIGPROFs during atomic64 and
postpone writing the sample(s) until called from elsewhere, with
pc set to _LostSIGPROFDuringAtomic64.

Added a test case, per Cherry's suggestion. Works around #20146.

Change-Id: Icff504180bae4ee83d78b19c0d9d6a80097087f9
Reviewed-on: https://go-review.googlesource.com/42652
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
  • Loading branch information
Vladimir Stefanovic authored and cherrymui committed Jul 26, 2017
1 parent df91b80 commit 835dfef
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 4 deletions.
9 changes: 9 additions & 0 deletions src/runtime/cpuprof.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,15 @@ func (p *cpuProfile) addExtra() {
}
}

func (p *cpuProfile) addLostAtomic64(count uint64) {
hdr := [1]uint64{count}
lostStk := [2]uintptr{
funcPC(_LostSIGPROFDuringAtomic64) + sys.PCQuantum,
funcPC(_System) + sys.PCQuantum,
}
cpuprof.log.write(nil, 0, hdr[:], lostStk[:])
}

// CPUProfile panics.
// It formerly provided raw access to chunks of
// a pprof-format profile generated by the runtime.
Expand Down
31 changes: 31 additions & 0 deletions src/runtime/pprof/pprof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"fmt"
"internal/testenv"
"io"
"io/ioutil"
"math/big"
"os"
"os/exec"
Expand All @@ -20,6 +21,7 @@ import (
"runtime/pprof/internal/profile"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
)
Expand Down Expand Up @@ -713,3 +715,32 @@ func TestCPUProfileLabel(t *testing.T) {
})
})
}

// Check that there is no deadlock when the program receives SIGPROF while in
// 64bit atomics' critical section. Used to happen on mips{,le}. See #20146.
func TestAtomicLoadStore64(t *testing.T) {
f, err := ioutil.TempFile("", "profatomic")
if err != nil {
t.Fatalf("TempFile: %v", err)
}
defer os.Remove(f.Name())
defer f.Close()

if err := StartCPUProfile(f); err != nil {
t.Fatal(err)
}
defer StopCPUProfile()

var flag uint64
done := make(chan bool, 1)

go func() {
for atomic.LoadUint64(&flag) == 0 {
runtime.Gosched()
}
done <- true
}()
time.Sleep(50 * time.Millisecond)
atomic.StoreUint64(&flag, 1)
<-done
}
31 changes: 27 additions & 4 deletions src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3232,10 +3232,14 @@ var prof struct {
hz int32
}

func _System() { _System() }
func _ExternalCode() { _ExternalCode() }
func _LostExternalCode() { _LostExternalCode() }
func _GC() { _GC() }
func _System() { _System() }
func _ExternalCode() { _ExternalCode() }
func _LostExternalCode() { _LostExternalCode() }
func _GC() { _GC() }
func _LostSIGPROFDuringAtomic64() { _LostSIGPROFDuringAtomic64() }

// Counts SIGPROFs received while in atomic64 critical section, on mips{,le}
var lostAtomic64Count uint64

// Called if we receive a SIGPROF signal.
// Called by the signal handler, may run during STW.
Expand All @@ -3245,6 +3249,21 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
return
}

// On mips{,le}, 64bit atomics are emulated with spinlocks, in
// runtime/internal/atomic. If SIGPROF arrives while the program is inside
// the critical section, it creates a deadlock (when writing the sample).
// As a workaround, create a counter of SIGPROFs while in critical section
// to store the count, and pass it to sigprof.add() later when SIGPROF is
// received from somewhere else (with _LostSIGPROFDuringAtomic64 as pc).
if GOARCH == "mips" || GOARCH == "mipsle" {
if f := findfunc(pc); f.valid() {
if hasprefix(funcname(f), "runtime/internal/atomic") {
lostAtomic64Count++
return
}
}
}

// Profiling runs concurrently with GC, so it must not allocate.
// Set a trap in case the code does allocate.
// Note that on windows, one thread takes profiles of all the
Expand Down Expand Up @@ -3371,6 +3390,10 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
}

if prof.hz != 0 {
if (GOARCH == "mips" || GOARCH == "mipsle") && lostAtomic64Count > 0 {
cpuprof.addLostAtomic64(lostAtomic64Count)
lostAtomic64Count = 0
}
cpuprof.add(gp, stk[:n])
}
getg().m.mallocing--
Expand Down

0 comments on commit 835dfef

Please sign in to comment.