From aefaeb75f3eff323f212c5309d8ae65768ad9809 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 5 Nov 2014 14:42:54 -0500 Subject: [dev.garbage] cmd/gc, runtime: add locks around print statements Now each C printf, Go print, or Go println is guaranteed not to be interleaved with other calls of those functions. This should help when debugging concurrent failures. LGTM=rlh R=rlh CC=golang-codereviews https://codereview.appspot.com/169120043 --- src/cmd/gc/builtin.c | 2 ++ src/cmd/gc/go.h | 1 + src/cmd/gc/runtime.go | 2 ++ src/cmd/gc/walk.c | 17 +++++++++++++++++ src/runtime/print1.go | 30 +++++++++++++++++++++++++++--- src/runtime/runtime.h | 1 + 6 files changed, 50 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/cmd/gc/builtin.c b/src/cmd/gc/builtin.c index bd3fca167..aeeadedca 100644 --- a/src/cmd/gc/builtin.c +++ b/src/cmd/gc/builtin.c @@ -24,6 +24,8 @@ char *runtimeimport = "func @\"\".printslice (? any)\n" "func @\"\".printnl ()\n" "func @\"\".printsp ()\n" + "func @\"\".printlock ()\n" + "func @\"\".printunlock ()\n" "func @\"\".concatstring2 (? string, ? string) (? string)\n" "func @\"\".concatstring3 (? string, ? string, ? string) (? string)\n" "func @\"\".concatstring4 (? string, ? string, ? string, ? string) (? string)\n" diff --git a/src/cmd/gc/go.h b/src/cmd/gc/go.h index 965a0550d..cc590416b 100644 --- a/src/cmd/gc/go.h +++ b/src/cmd/gc/go.h @@ -1464,6 +1464,7 @@ void walk(Node *fn); void walkexpr(Node **np, NodeList **init); void walkexprlist(NodeList *l, NodeList **init); void walkexprlistsafe(NodeList *l, NodeList **init); +void walkexprlistcheap(NodeList *l, NodeList **init); void walkstmt(Node **np); void walkstmtlist(NodeList *l); Node* conv(Node*, Type*); diff --git a/src/cmd/gc/runtime.go b/src/cmd/gc/runtime.go index 38bf6abb6..c6007714c 100644 --- a/src/cmd/gc/runtime.go +++ b/src/cmd/gc/runtime.go @@ -36,6 +36,8 @@ func printeface(any) func printslice(any) func printnl() func printsp() +func printlock() +func printunlock() func concatstring2(string, string) string func concatstring3(string, string, string) string diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c index 7b502eb60..38bed1e22 100644 --- a/src/cmd/gc/walk.c +++ b/src/cmd/gc/walk.c @@ -363,6 +363,15 @@ walkexprlistsafe(NodeList *l, NodeList **init) } } +void +walkexprlistcheap(NodeList *l, NodeList **init) +{ + for(; l; l=l->next) { + l->n = cheapexpr(l->n, init); + walkexpr(&l->n, init); + } +} + void walkexpr(Node **np, NodeList **init) { @@ -1773,6 +1782,11 @@ walkprint(Node *nn, NodeList **init) calls = nil; notfirst = 0; + // Hoist all the argument evaluation up before the lock. + walkexprlistcheap(all, init); + + calls = list(calls, mkcall("printlock", T, init)); + for(l=all; l; l=l->next) { if(notfirst) { calls = list(calls, mkcall("printsp", T, init)); @@ -1853,6 +1867,9 @@ walkprint(Node *nn, NodeList **init) if(op == OPRINTN) calls = list(calls, mkcall("printnl", T, nil)); + + calls = list(calls, mkcall("printunlock", T, init)); + typechecklist(calls, Etop); walkexprlist(calls, init); diff --git a/src/runtime/print1.go b/src/runtime/print1.go index 8f8268873..3d812bd04 100644 --- a/src/runtime/print1.go +++ b/src/runtime/print1.go @@ -41,7 +41,31 @@ func snprintf(dst *byte, n int32, s *byte) { gp.writebuf = nil } -//var debuglock mutex +var debuglock mutex + +// The compiler emits calls to printlock and printunlock around +// the multiple calls that implement a single Go print or println +// statement. Some of the print helpers (printsp, for example) +// call print recursively. There is also the problem of a crash +// happening during the print routines and needing to acquire +// the print lock to print information about the crash. +// For both these reasons, let a thread acquire the printlock 'recursively'. + +func printlock() { + mp := getg().m + mp.printlock++ + if mp.printlock == 1 { + lock(&debuglock) + } +} + +func printunlock() { + mp := getg().m + mp.printlock-- + if mp.printlock == 0 { + unlock(&debuglock) + } +} // write to goroutine-local buffer if diverting output, // or else standard error. @@ -80,7 +104,7 @@ func printnl() { // Very simple printf. Only for debugging prints. // Do not add to this without checking with Rob. func vprintf(str string, arg unsafe.Pointer) { - //lock(&debuglock); + printlock() s := bytes(str) start := 0 @@ -160,7 +184,7 @@ func vprintf(str string, arg unsafe.Pointer) { gwrite(s[start:i]) } - //unlock(&debuglock); + printunlock() } func printpc(p unsafe.Pointer) { diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 6a02ef1d3..ee86f2d17 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -345,6 +345,7 @@ struct M int32 helpgc; bool spinning; // M is out of work and is actively looking for work bool blocked; // M is blocked on a Note + int8 printlock; uint32 fastrand; uint64 ncgocall; // number of cgo calls in total int32 ncgo; // number of cgo calls currently in progress -- cgit v1.2.1