From 3fec6da0ab544bfa3a3b3d9988df20739b40dd49 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 21 Nov 2017 15:24:12 -0800 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/go/build/deps_test.go | 3 + src/internal/poll/fd_unix.go | 7 ++ src/os/signal/internal/pty/pty.go | 42 +++++++++ src/os/signal/signal_cgo_test.go | 145 ++++++++++++++++++++++++++++++ 4 files changed, 197 insertions(+) create mode 100644 src/os/signal/internal/pty/pty.go create mode 100644 src/os/signal/signal_cgo_test.go diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 5ab4cedd51..4169cb7780 100644 --- 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 3ac6927337..7d95c8d68f 100644 --- 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 index 0000000000..704af3f67b --- /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 +#include +#include +*/ +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 index 0000000000..6eed979ab0 --- /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) + } +} -- 2.50.0