internal/poll: loop on EINTR in Read on Darwin

Test is in os/signal package because the problem is signal related.

Fixes #22838.

Change-Id: I223eeebb5fbc972910737eddef8ab9784cb984a6
Reviewed-on: https://go-review.googlesource.com/79215
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/src/go/build/deps_test.go b/src/go/build/deps_test.go
index 5ab4cedd5170fdb06593fd796f4eebae2e510744..4169cb7780d96bb05fc0ead63dd8cefab636e983 100644 (file)
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -299,6 +299,9 @@ var pkgDeps = map[string][]string{
        // Plan 9 alone needs io/ioutil and os.
        "os/user": {"L4", "CGO", "io/ioutil", "os", "syscall"},
 
+       // Internal package used only for testing.
+       "os/signal/internal/pty": {"CGO", "fmt", "os"},
+
        // Basic networking.
        // Because net must be used by any package that wants to
        // do networking portably, it must have a small dependency set: just L0+basic os.

diff --git a/src/internal/poll/fd_unix.go b/src/internal/poll/fd_unix.go
index 3ac69273370e1e6371999b8200776609199d46fb..7d95c8d68f8977bcaa10660aeb4727af98fb8e98 100644 (file)
--- a/src/internal/poll/fd_unix.go
+++ b/src/internal/poll/fd_unix.go
@@ -8,6 +8,7 @@ package poll
 
 import (
        "io"
+       "runtime"
        "syscall"
 )
 
@@ -135,6 +136,12 @@ func (fd *FD) Read(p []byte) (int, error) {
                                        continue
                                }
                        }
+
+                       // On MacOS we can see EINTR here if the user
+                       // pressed ^Z.  See issue #22838.
+                       if runtime.GOOS == "darwin" && err == syscall.EINTR {
+                               continue
+                       }
                }
                err = fd.eofError(n, err)
                return n, err

diff --git a/src/os/signal/internal/pty/pty.go b/src/os/signal/internal/pty/pty.go
new file mode 100644 (file)
index 0000000..704af3f
--- /dev/null
+++ b/src/os/signal/internal/pty/pty.go
@@ -0,0 +1,42 @@
+// 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.
+
+// +build darwin dragonfly freebsd linux netbsd openbsd solaris
+// +build cgo
+
+// Package pty is a simple pseudo-terminal package for Unix systems,
+// implemented by calling C functions via cgo.
+// This is only used for testing the os/signal package.
+package pty
+
+/*
+#define _XOPEN_SOURCE 600
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>
+*/
+import "C"
+
+import (
+       "fmt"
+       "os"
+)
+
+// Open returns a master pty and the name of the linked slave tty.
+func Open() (master *os.File, slave string, err error) {
+       m, err := C.posix_openpt(C.O_RDWR)
+       if err != nil {
+               return nil, "", fmt.Errorf("posix_openpt: %v", err)
+       }
+       if _, err := C.grantpt(m); err != nil {
+               C.close(m)
+               return nil, "", fmt.Errorf("grantpt: %v", err)
+       }
+       if _, err := C.unlockpt(m); err != nil {
+               C.close(m)
+               return nil, "", fmt.Errorf("unlockpt: %v", err)
+       }
+       slave = C.GoString(C.ptsname(m))
+       return os.NewFile(uintptr(m), "pty-master"), slave, nil
+}

diff --git a/src/os/signal/signal_cgo_test.go b/src/os/signal/signal_cgo_test.go
new file mode 100644 (file)
index 0000000..6eed979
--- /dev/null
+++ b/src/os/signal/signal_cgo_test.go
@@ -0,0 +1,145 @@
+// 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.
+
+// +build darwin dragonfly freebsd linux netbsd openbsd solaris
+// +build cgo
+
+package signal_test
+
+import (
+       "bufio"
+       "context"
+       "fmt"
+       "io"
+       "os"
+       "os/exec"
+       "os/signal/internal/pty"
+       "strconv"
+       "strings"
+       "syscall"
+       "testing"
+       "time"
+)
+
+func TestTerminalSignal(t *testing.T) {
+       if os.Getenv("GO_TEST_TERMINAL_SIGNALS") != "" {
+               var b [1]byte
+               fmt.Println("entering read")
+               n, err := os.Stdin.Read(b[:])
+               fmt.Printf("read %d bytes: %q\n", n, b)
+               if err != nil {
+                       fmt.Println(err)
+                       os.Exit(1)
+               }
+               os.Exit(0)
+       }
+
+       // The test requires a shell that uses job control.
+       bash, err := exec.LookPath("bash")
+       if err != nil {
+               t.Skipf("could not find bash: %v", err)
+       }
+
+       scale := 1
+       if s := os.Getenv("GO_TEST_TIMEOUT_SCALE"); s != "" {
+               if sc, err := strconv.Atoi(s); err == nil {
+                       scale = sc
+               }
+       }
+
+       // The test only fails when using a "slow device," in this
+       // case a pseudo-terminal.
+
+       master, sname, err := pty.Open()
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer master.Close()
+       slave, err := os.OpenFile(sname, os.O_RDWR, 0)
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer slave.Close()
+
+       // Start an interactive shell.
+       ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+       defer cancel()
+       cmd := exec.CommandContext(ctx, bash, "-i")
+       cmd.Stdin = slave
+       cmd.Stdout = slave
+       cmd.Stderr = slave
+       cmd.SysProcAttr = &syscall.SysProcAttr{
+               Setsid:  true,
+               Setctty: true,
+               Ctty:    int(slave.Fd()),
+       }
+
+       if err := cmd.Start(); err != nil {
+               t.Fatal(err)
+       }
+
+       if err := slave.Close(); err != nil {
+               t.Errorf("closing slave: %v", err)
+       }
+
+       // Read data from master in the background.
+       go func() {
+               buf := bufio.NewReader(master)
+               for {
+                       data, err := buf.ReadBytes('\n')
+                       if len(data) > 0 {
+                               t.Logf("%q", data)
+                       }
+                       if err != nil {
+                               if perr, ok := err.(*os.PathError); ok {
+                                       err = perr.Err
+                               }
+                               // EOF means master is closed.
+                               // EIO means child process is done.
+                               // "file already closed" means deferred close of master has happened.
+                               if err != io.EOF && err != syscall.EIO && !strings.Contains(err.Error(), "file already closed") {
+                                       t.Logf("error reading from master: %v", err)
+                               }
+                               return
+                       }
+               }
+       }()
+
+       // Start a small program that reads from stdin
+       // (namely the code at the top of this function).
+       if _, err := master.Write([]byte("GO_TEST_TERMINAL_SIGNALS=1 " + os.Args[0] + " -test.run=TestTerminalSignal\n")); err != nil {
+               t.Fatal(err)
+       }
+
+       // Give the program time to enter the read call.
+       time.Sleep(time.Duration(scale) * 100 * time.Millisecond)
+
+       // Send a ^Z to stop the program.
+       if _, err := master.Write([]byte{26}); err != nil {
+               t.Fatalf("writing ^Z to pty: %v", err)
+       }
+
+       // Give the process time to handle the signal.
+       time.Sleep(time.Duration(scale) * 100 * time.Millisecond)
+
+       // Restart the stopped program.
+       if _, err := master.Write([]byte("fg\n")); err != nil {
+               t.Fatalf("writing %q to pty: %v", "fg", err)
+       }
+
+       // Write some data for the program to read,
+       // which should cause it to exit.
+       if _, err := master.Write([]byte{'\n'}); err != nil {
+               t.Fatalf("writing %q to pty: %v", "\n", err)
+       }
+
+       // Exit the shell with the program's exit status.
+       if _, err := master.Write([]byte("exit $?\n")); err != nil {
+               t.Fatalf("writing %q to pty: %v", "exit", err)
+       }
+
+       if err = cmd.Wait(); err != nil {
+               t.Errorf("subprogram failed: %v", err)
+       }
+}