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 <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/misc/cgo/testsanitizers/test.bash b/misc/cgo/testsanitizers/test.bash
index 67925e52ee48457e2017ab8cdba1471972bff239..556ef108205dc1854b02646d73e1728fc076d458 100755 (executable)
--- 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 (file)
index 0000000..70ac9c8
--- /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 <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+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 341c74ff39b6234b611c6fa5966f09e31c18efb7..c3ed3be00b92f569bf42d2dc3eea4d0653424a48 100644 (file)
--- 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 e00b99164f4b1e30efd13644509db92df5fcb301..4a8295ff4722b1e0a5b41d726067ba7e4f86c74b 100644 (file)
--- 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 5b907502d645728ead6955fb14643cd6e9598456..caeb51205b0ce53500fea256a6931df3bd864b08 100644 (file)
--- 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()