From: Bryan C. Mills Date: Wed, 8 Mar 2017 20:40:28 +0000 (-0500) Subject: runtime: fix _cgo_yield usage with sysmon and on BSD X-Git-Tag: go1.9beta1~1223 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=e57350f4c091c80333d37b2ba5af50de193147fa;p=gostls13.git runtime: fix _cgo_yield usage with sysmon and on BSD There are a few problems from change 35494, discovered during testing of change 37852. 1. I was confused about the usage of n.key in the sema variant, so we were looping on the wrong condition. The error was not caught by the TryBots (presumably due to missing TSAN coverage in the BSD and darwin builders?). 2. The sysmon goroutine sometimes skips notetsleep entirely, using direct usleep syscalls instead. In that case, we were not calling _cgo_yield, leading to missed signals under TSAN. 3. Some notetsleep calls have long finite timeouts. They should be broken up into smaller chunks with a yield at the end of each chunk. updates #18717 Change-Id: I91175af5dea3857deebc686f51a8a40f9d690bcc Reviewed-on: https://go-review.googlesource.com/37867 Run-TryBot: Bryan Mills TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- diff --git a/misc/cgo/testsanitizers/test.bash b/misc/cgo/testsanitizers/test.bash index 67925e52ee..556ef10820 100755 --- a/misc/cgo/testsanitizers/test.bash +++ b/misc/cgo/testsanitizers/test.bash @@ -190,17 +190,12 @@ if test "$tsan" = "yes"; then fi if test "$ok" = "true"; then - # This test requires rebuilding os/user with -fsanitize=thread. + # These tests require rebuilding os/user with -fsanitize=thread. testtsan tsan5.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan" - - # This test requires rebuilding runtime/cgo with -fsanitize=thread. testtsan tsan6.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan" - - # This test requires rebuilding runtime/cgo with -fsanitize=thread. testtsan tsan7.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan" - - # This test requires rebuilding runtime/cgo with -fsanitize=thread. testtsan tsan10.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan" + testtsan tsan11.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan" fi fi diff --git a/misc/cgo/testsanitizers/tsan11.go b/misc/cgo/testsanitizers/tsan11.go new file mode 100644 index 0000000000..70ac9c8ae2 --- /dev/null +++ b/misc/cgo/testsanitizers/tsan11.go @@ -0,0 +1,55 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +// This program hung when run under the C/C++ ThreadSanitizer. TSAN defers +// asynchronous signals until the signaled thread calls into libc. The runtime's +// sysmon goroutine idles itself using direct usleep syscalls, so it could +// run for an arbitrarily long time without triggering the libc interceptors. +// See https://golang.org/issue/18717. + +import ( + "os" + "os/signal" + "syscall" +) + +/* +#cgo CFLAGS: -g -fsanitize=thread +#cgo LDFLAGS: -g -fsanitize=thread + +#include +#include +#include +#include + +static void raise_usr2(int signo) { + raise(SIGUSR2); +} + +static void register_handler(int signo) { + struct sigaction sa; + memset(&sa, 0, sizeof(sa)); + sigemptyset(&sa.sa_mask); + sa.sa_flags = SA_ONSTACK; + sa.sa_handler = raise_usr2; + + if (sigaction(SIGUSR1, &sa, NULL) != 0) { + perror("failed to register SIGUSR1 handler"); + exit(EXIT_FAILURE); + } +} +*/ +import "C" + +func main() { + ch := make(chan os.Signal) + signal.Notify(ch, syscall.SIGUSR2) + + C.register_handler(C.int(syscall.SIGUSR1)) + syscall.Kill(syscall.Getpid(), syscall.SIGUSR1) + + <-ch +} diff --git a/src/runtime/lock_futex.go b/src/runtime/lock_futex.go index 341c74ff39..c3ed3be00b 100644 --- a/src/runtime/lock_futex.go +++ b/src/runtime/lock_futex.go @@ -185,8 +185,14 @@ func notetsleep_internal(n *note, ns int64) bool { deadline := nanotime() + ns for { + if _cgo_yield != nil && ns > 10e6 { + ns = 10e6 + } gp.m.blocked = true futexsleep(key32(&n.key), 0, ns) + if _cgo_yield != nil { + asmcgocall(_cgo_yield, nil) + } gp.m.blocked = false if atomic.Load(key32(&n.key)) != 0 { break diff --git a/src/runtime/lock_sema.go b/src/runtime/lock_sema.go index e00b99164f..4a8295ff47 100644 --- a/src/runtime/lock_sema.go +++ b/src/runtime/lock_sema.go @@ -198,10 +198,9 @@ func notetsleep_internal(n *note, ns int64, gp *g, deadline int64) bool { if _cgo_yield == nil { semasleep(-1) } else { - // Sleep for an arbitrary-but-moderate interval to poll libc interceptors. + // Sleep in arbitrary-but-moderate intervals to poll libc interceptors. const ns = 10e6 - for atomic.Loaduintptr(&n.key) == 0 { - semasleep(ns) + for semasleep(ns) < 0 { asmcgocall(_cgo_yield, nil) } } @@ -213,12 +212,18 @@ func notetsleep_internal(n *note, ns int64, gp *g, deadline int64) bool { for { // Registered. Sleep. gp.m.blocked = true + if _cgo_yield != nil && ns > 10e6 { + ns = 10e6 + } if semasleep(ns) >= 0 { gp.m.blocked = false // Acquired semaphore, semawakeup unregistered us. // Done. return true } + if _cgo_yield != nil { + asmcgocall(_cgo_yield, nil) + } gp.m.blocked = false // Interrupted or timed out. Still registered. Semaphore not acquired. ns = deadline - nanotime() diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 5b907502d6..caeb51205b 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -3755,6 +3755,10 @@ func sysmon() { } unlock(&sched.lock) } + // trigger libc interceptors if needed + if _cgo_yield != nil { + asmcgocall(_cgo_yield, nil) + } // poll network if not polled for more than 10ms lastpoll := int64(atomic.Load64(&sched.lastpoll)) now := nanotime()