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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>

Author: Bryan C. Mills

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
 

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
+}

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

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()

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()