misc/cgo/testcarchive: more robust TestSignalForwardingExternal
Try to avoid a race condition in the test. Passed 500 times on my
laptop.
Fixes #14956.
Change-Id: I5de2e1e3623832f0ab4f180149f7c57ce7cd23c0
Reviewed-on: https://go-review.googlesource.com/21171
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/misc/cgo/testcarchive/carchive_test.go b/misc/cgo/testcarchive/carchive_test.go
index 2c81a6a..97e0c78 100644
--- a/misc/cgo/testcarchive/carchive_test.go
+++ b/misc/cgo/testcarchive/carchive_test.go
@@ -14,6 +14,7 @@
"strings"
"syscall"
"testing"
+ "time"
"unicode"
)
@@ -312,44 +313,65 @@
t.Fatal(err)
}
- cmd = exec.Command(bin[0], append(bin[1:], "2")...)
+ // We want to send the process a signal and see if it dies.
+ // Normally the signal goes to the C thread, the Go signal
+ // handler picks it up, sees that it is running in a C thread,
+ // and the program dies. Unfortunately, occasionally the
+ // signal is delivered to a Go thread, which winds up
+ // discarding it because it was sent by another program and
+ // there is no Go handler for it. To avoid this, run the
+ // program several times in the hopes that it will eventually
+ // fail.
+ const tries = 20
+ for i := 0; i < tries; i++ {
+ cmd = exec.Command(bin[0], append(bin[1:], "2")...)
- stderr, err := cmd.StderrPipe()
- if err != nil {
- t.Fatal(err)
- }
- defer stderr.Close()
+ stderr, err := cmd.StderrPipe()
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer stderr.Close()
- r := bufio.NewReader(stderr)
+ r := bufio.NewReader(stderr)
- err = cmd.Start()
+ err = cmd.Start()
- if err != nil {
- t.Fatal(err)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ // Wait for trigger to ensure that the process is started.
+ ok, err := r.ReadString('\n')
+
+ // Verify trigger.
+ if err != nil || ok != "OK\n" {
+ t.Fatalf("Did not receive OK signal")
+ }
+
+ // Give the program a chance to enter the sleep function.
+ time.Sleep(time.Millisecond)
+
+ cmd.Process.Signal(syscall.SIGSEGV)
+
+ err = cmd.Wait()
+
+ if err == nil {
+ continue
+ }
+
+ if ee, ok := err.(*exec.ExitError); !ok {
+ t.Errorf("error (%v) has type %T; expected exec.ExitError", err, err)
+ } else if ws, ok := ee.Sys().(syscall.WaitStatus); !ok {
+ t.Errorf("error.Sys (%v) has type %T; expected syscall.WaitStatus", ee.Sys(), ee.Sys())
+ } else if !ws.Signaled() || ws.Signal() != syscall.SIGSEGV {
+ t.Errorf("got %v; expected SIGSEGV", ee)
+ } else {
+ // We got the error we expected.
+ return
+ }
}
- // Wait for trigger to ensure that the process is started.
- ok, err := r.ReadString('\n')
-
- // Verify trigger.
- if err != nil || ok != "OK\n" {
- t.Fatalf("Did not receive OK signal")
- }
-
- // Trigger an interrupt external to the process.
- cmd.Process.Signal(syscall.SIGSEGV)
-
- err = cmd.Wait()
-
- if err == nil {
- t.Error("test program succeeded unexpectedly")
- } else if ee, ok := err.(*exec.ExitError); !ok {
- t.Errorf("error (%v) has type %T; expected exec.ExitError", err, err)
- } else if ws, ok := ee.Sys().(syscall.WaitStatus); !ok {
- t.Errorf("error.Sys (%v) has type %T; expected syscall.WaitStatus", ee.Sys(), ee.Sys())
- } else if !ws.Signaled() || ws.Signal() != syscall.SIGSEGV {
- t.Errorf("got %v; expected SIGSEGV", ee)
- }
+ t.Errorf("program succeeded unexpectedly %d times", tries)
}
func TestOsSignal(t *testing.T) {
diff --git a/misc/cgo/testcarchive/main5.c b/misc/cgo/testcarchive/main5.c
index 556abdf..9fadf08 100644
--- a/misc/cgo/testcarchive/main5.c
+++ b/misc/cgo/testcarchive/main5.c
@@ -8,6 +8,9 @@
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
+#include <sys/types.h>
+#include <sys/time.h>
+#include <sys/select.h>
#include "libgo2.h"
@@ -16,7 +19,7 @@
int test;
if (argc < 2) {
- printf("Missing argument");
+ printf("Missing argument\n");
return 1;
}
@@ -28,7 +31,7 @@
printf("calling RunGoroutines\n");
}
- RunGoroutines();
+ Noop();
switch (test) {
case 1: {
@@ -41,6 +44,8 @@
}
case 2: {
+ struct timeval tv;
+
if (verbose) {
printf("attempting external signal test\n");
}
@@ -48,8 +53,18 @@
fprintf(stderr, "OK\n");
fflush(stderr);
- // The program should be interrupted before this sleep finishes.
- sleep(60);
+ // The program should be interrupted before
+ // this sleep finishes. We use select rather
+ // than sleep because in older versions of
+ // glibc the sleep function does some signal
+ // fiddling to handle SIGCHLD. If this
+ // program is fiddling signals just when the
+ // test program sends the signal, the signal
+ // may be delivered to a Go thread which will
+ // break this test.
+ tv.tv_sec = 60;
+ tv.tv_usec = 0;
+ select(0, NULL, NULL, NULL, &tv);
break;
}
diff --git a/misc/cgo/testcarchive/src/libgo2/libgo2.go b/misc/cgo/testcarchive/src/libgo2/libgo2.go
index ab40b75..fbed493 100644
--- a/misc/cgo/testcarchive/src/libgo2/libgo2.go
+++ b/misc/cgo/testcarchive/src/libgo2/libgo2.go
@@ -41,5 +41,10 @@
os.Exit(1)
}
+// Noop ensures that the Go runtime is initialized.
+//export Noop
+func Noop() {
+}
+
func main() {
}
