Skip to content

Commit

Permalink
runtime: debug code to catch bad gcWork.puts
Browse files Browse the repository at this point in the history
This adds a debug check to throw immediately if any pointers are added
to the gcWork buffer after the mark completion barrier. The intent is
to catch the source of the cached GC work that occasionally produces
"P has cached GC work at end of mark termination" failures.

The result should be that we get "throwOnGCWork" throws instead of "P
has cached GC work at end of mark termination" throws, but with useful
stack traces.

This should be reverted before the release. I've been unable to
reproduce this issue locally, but this issue appears fairly regularly
on the builders, so the intent is to catch it on the builders.

This probably slows down the GC slightly.

For #27993.

Change-Id: I5035e14058ad313bfbd3d68c41ec05179147a85c
Reviewed-on: https://go-review.googlesource.com/c/149969
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
  • Loading branch information
aclements committed Nov 21, 2018
1 parent b8fad4b commit 9098d1d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/gc/inl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestIntendedInlining(t *testing.T) {
"puintptr.ptr",
"spanOf",
"spanOfUnchecked",
"(*gcWork).putFast",
//"(*gcWork).putFast", // TODO(austin): For debugging #27993
"(*gcWork).tryGetFast",
"(*guintptr).set",
"(*markBits).advance",
Expand Down
6 changes: 5 additions & 1 deletion src/runtime/mgc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,8 @@ top:
goto top
}

throwOnGCWork = true

// There was no global work, no local work, and no Ps
// communicated work since we took markDoneSema. Therefore
// there are no grey objects and no more objects can be
Expand Down Expand Up @@ -1924,7 +1926,7 @@ func gcMark(start_time int64) {
// ensured all reachable objects were marked, all of
// these must be pointers to black objects. Hence we
// can just discard the write barrier buffer.
if debug.gccheckmark > 0 {
if debug.gccheckmark > 0 || throwOnGCWork {
// For debugging, flush the buffer and make
// sure it really was all marked.
wbBufFlush1(p)
Expand Down Expand Up @@ -1956,6 +1958,8 @@ func gcMark(start_time int64) {
gcw.dispose()
}

throwOnGCWork = false

cachestats()

// Update the marked heap stat.
Expand Down
25 changes: 25 additions & 0 deletions src/runtime/mgcwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ const (
workbufAlloc = 32 << 10
)

// throwOnGCWork causes any operations that add pointers to a gcWork
// buffer to throw.
//
// TODO(austin): This is a temporary debugging measure for issue
// #27993. To be removed before release.
var throwOnGCWork bool

func init() {
if workbufAlloc%pageSize != 0 || workbufAlloc%_WorkbufSize != 0 {
throw("bad workbufAlloc")
Expand Down Expand Up @@ -108,6 +115,10 @@ func (w *gcWork) init() {
// obj must point to the beginning of a heap object or an oblet.
//go:nowritebarrierrec
func (w *gcWork) put(obj uintptr) {
if throwOnGCWork {
throw("throwOnGCWork")
}

flushed := false
wbuf := w.wbuf1
if wbuf == nil {
Expand Down Expand Up @@ -142,6 +153,10 @@ func (w *gcWork) put(obj uintptr) {
// otherwise it returns false and the caller needs to call put.
//go:nowritebarrierrec
func (w *gcWork) putFast(obj uintptr) bool {
if throwOnGCWork {
throw("throwOnGCWork")
}

wbuf := w.wbuf1
if wbuf == nil {
return false
Expand All @@ -163,6 +178,10 @@ func (w *gcWork) putBatch(obj []uintptr) {
return
}

if throwOnGCWork {
throw("throwOnGCWork")
}

flushed := false
wbuf := w.wbuf1
if wbuf == nil {
Expand Down Expand Up @@ -284,10 +303,16 @@ func (w *gcWork) balance() {
return
}
if wbuf := w.wbuf2; wbuf.nobj != 0 {
if throwOnGCWork {
throw("throwOnGCWork")
}
putfull(wbuf)
w.flushedWork = true
w.wbuf2 = getempty()
} else if wbuf := w.wbuf1; wbuf.nobj > 4 {
if throwOnGCWork {
throw("throwOnGCWork")
}
w.wbuf1 = handoff(wbuf)
w.flushedWork = true // handoff did putfull
} else {
Expand Down

0 comments on commit 9098d1d

Please sign in to comment.