diff --git a/io_test.go b/io_test.go new file mode 100644 index 0000000..0837d9e --- /dev/null +++ b/io_test.go @@ -0,0 +1,119 @@ +//go:build go1.12 +// +build go1.12 + +package pty + +import ( + "testing" + + "context" + "errors" + "os" + "runtime" + "sync" + "time" +) + +const ( + errMarker byte = 0xEE + timeout = time.Second +) + +var mu sync.Mutex + +// Check that SetDeadline() works for ptmx. +// Outstanding Read() calls must be interrupted by deadline. +// +// https://github.com/creack/pty/issues/162 +func TestReadDeadline(t *testing.T) { + ptmx, success := prepare(t) + + err := ptmx.SetDeadline(time.Now().Add(timeout / 10)) + if err != nil { + if errors.Is(err, os.ErrNoDeadline) { + t.Skipf("deadline is not supported on %s/%s", runtime.GOOS, runtime.GOARCH) + } else { + t.Fatalf("error: set deadline: %v\n", err) + } + } + + var buf = make([]byte, 1) + n, err := ptmx.Read(buf) + success() + + if n != 0 && buf[0] != errMarker { + t.Errorf("received unexpected data from pmtx (%d bytes): 0x%X; err=%v", n, buf, err) + } +} + +// Check that ptmx.Close() interrupts outstanding ptmx.Read() calls +// +// https://github.com/creack/pty/issues/114 +// https://github.com/creack/pty/issues/88 +func TestReadClose(t *testing.T) { + ptmx, success := prepare(t) + + go func() { + time.Sleep(timeout / 10) + err := ptmx.Close() + if err != nil { + t.Errorf("failed to close ptmx: %v", err) + } + }() + + var buf = make([]byte, 1) + n, err := ptmx.Read(buf) + success() + + if n != 0 && buf[0] != errMarker { + t.Errorf("received unexpected data from pmtx (%d bytes): 0x%X; err=%v", n, buf, err) + } +} + +// Open pty and setup watchdogs for graceful and not so graceful failure modes +func prepare(t *testing.T) (ptmx *os.File, done func()) { + if runtime.GOOS == "darwin" { + t.Log("creack/pty uses blocking i/o on darwin intentionally:") + t.Log("> https://github.com/creack/pty/issues/52") + t.Log("> https://github.com/creack/pty/pull/53") + t.Log("> https://github.com/golang/go/issues/22099") + t.SkipNow() + } + + // Due to data race potential in (*os.File).Fd() + // we should never run these two tests in parallel + mu.Lock() + t.Cleanup(mu.Unlock) + + ptmx, pts, err := Open() + if err != nil { + t.Fatalf("error: open: %v\n", err) + } + t.Cleanup(func() { _ = ptmx.Close() }) + t.Cleanup(func() { _ = pts.Close() }) + + ctx, done := context.WithCancel(context.Background()) + t.Cleanup(done) + go func() { + select { + case <-ctx.Done(): + // ptmx.Read() did not block forever, yay! + case <-time.After(timeout): + _, err := pts.Write([]byte{errMarker}) // unblock ptmx.Read() + if err != nil { + t.Errorf("failed to write to pts: %v", err) + } + t.Error("ptmx.Read() was not unblocked") + done() // cancel panic() + } + }() + go func() { + select { + case <-ctx.Done(): + // Test has either failed or succeeded; it definitely did not hang + case <-time.After(timeout * 10 / 9): // timeout +11% + panic("ptmx.Read() was not unblocked; avoid hanging forever") // just in case + } + }() + return ptmx, done +} diff --git a/ioctl.go b/ioctl.go index 3cabedd..60ac9b8 100644 --- a/ioctl.go +++ b/ioctl.go @@ -1,19 +1,23 @@ -//go:build !windows && !solaris && !aix -// +build !windows,!solaris,!aix +//go:build !windows && go1.12 +// +build !windows,go1.12 package pty -import "syscall" +import "os" -const ( - TIOCGWINSZ = syscall.TIOCGWINSZ - TIOCSWINSZ = syscall.TIOCSWINSZ -) +func ioctl(f *os.File, cmd, ptr uintptr) error { + sc, e := f.SyscallConn() + if e != nil { + return ioctl_inner(f.Fd(), cmd, ptr) // fall back to blocking io (old behavior) + } + + ch := make(chan error, 1) + defer close(ch) -func ioctl(fd, cmd, ptr uintptr) error { - _, _, e := syscall.Syscall(syscall.SYS_IOCTL, fd, cmd, ptr) - if e != 0 { + e = sc.Control(func(fd uintptr) { ch <- ioctl_inner(fd, cmd, ptr) }) + if e != nil { return e } - return nil + e = <-ch + return e } diff --git a/ioctl_inner.go b/ioctl_inner.go new file mode 100644 index 0000000..fd5dbef --- /dev/null +++ b/ioctl_inner.go @@ -0,0 +1,19 @@ +//go:build !windows && !solaris && !aix +// +build !windows,!solaris,!aix + +package pty + +import "syscall" + +const ( + TIOCGWINSZ = syscall.TIOCGWINSZ + TIOCSWINSZ = syscall.TIOCSWINSZ +) + +func ioctl_inner(fd, cmd, ptr uintptr) error { + _, _, e := syscall.Syscall(syscall.SYS_IOCTL, fd, cmd, ptr) + if e != 0 { + return e + } + return nil +} diff --git a/ioctl_legacy.go b/ioctl_legacy.go new file mode 100644 index 0000000..f00b1a1 --- /dev/null +++ b/ioctl_legacy.go @@ -0,0 +1,10 @@ +//go:build !windows && !go1.12 +// +build !windows,!go1.12 + +package pty + +import "os" + +func ioctl(f *os.File, cmd, ptr uintptr) error { + return ioctl_inner(f.Fd(), cmd, ptr) // fall back to blocking io (old behavior) +} diff --git a/ioctl_solaris.go b/ioctl_solaris.go index bff22da..c0231df 100644 --- a/ioctl_solaris.go +++ b/ioctl_solaris.go @@ -40,7 +40,7 @@ type strioctl struct { // Defined in asm_solaris_amd64.s. func sysvicall6(trap, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err syscall.Errno) -func ioctl(fd, cmd, ptr uintptr) error { +func ioctl_inner(fd, cmd, ptr uintptr) error { if _, _, errno := sysvicall6(uintptr(unsafe.Pointer(&procioctl)), 3, fd, cmd, ptr, 0, 0, 0); errno != 0 { return errno } diff --git a/ioctl_unsupported.go b/ioctl_unsupported.go index 2449a27..79ad4c6 100644 --- a/ioctl_unsupported.go +++ b/ioctl_unsupported.go @@ -8,6 +8,6 @@ const ( TIOCSWINSZ = 0 ) -func ioctl(fd, cmd, ptr uintptr) error { +func ioctl_inner(fd, cmd, ptr uintptr) error { return ErrUnsupported } diff --git a/pty_darwin.go b/pty_darwin.go index 9bdd71d..eadf6ab 100644 --- a/pty_darwin.go +++ b/pty_darwin.go @@ -46,7 +46,7 @@ func open() (pty, tty *os.File, err error) { func ptsname(f *os.File) (string, error) { n := make([]byte, _IOC_PARM_LEN(syscall.TIOCPTYGNAME)) - err := ioctl(f.Fd(), syscall.TIOCPTYGNAME, uintptr(unsafe.Pointer(&n[0]))) + err := ioctl(f, syscall.TIOCPTYGNAME, uintptr(unsafe.Pointer(&n[0]))) if err != nil { return "", err } @@ -60,9 +60,9 @@ func ptsname(f *os.File) (string, error) { } func grantpt(f *os.File) error { - return ioctl(f.Fd(), syscall.TIOCPTYGRANT, 0) + return ioctl(f, syscall.TIOCPTYGRANT, 0) } func unlockpt(f *os.File) error { - return ioctl(f.Fd(), syscall.TIOCPTYUNLK, 0) + return ioctl(f, syscall.TIOCPTYUNLK, 0) } diff --git a/pty_dragonfly.go b/pty_dragonfly.go index aa916aa..12803de 100644 --- a/pty_dragonfly.go +++ b/pty_dragonfly.go @@ -45,17 +45,17 @@ func open() (pty, tty *os.File, err error) { } func grantpt(f *os.File) error { - _, err := isptmaster(f.Fd()) + _, err := isptmaster(f) return err } func unlockpt(f *os.File) error { - _, err := isptmaster(f.Fd()) + _, err := isptmaster(f) return err } -func isptmaster(fd uintptr) (bool, error) { - err := ioctl(fd, syscall.TIOCISPTMASTER, 0) +func isptmaster(f *os.File) (bool, error) { + err := ioctl(f, syscall.TIOCISPTMASTER, 0) return err == nil, err } @@ -68,7 +68,7 @@ func ptsname(f *os.File) (string, error) { name := make([]byte, _C_SPECNAMELEN) fa := fiodgnameArg{Name: (*byte)(unsafe.Pointer(&name[0])), Len: _C_SPECNAMELEN, Pad_cgo_0: [4]byte{0, 0, 0, 0}} - err := ioctl(f.Fd(), ioctl_FIODNAME, uintptr(unsafe.Pointer(&fa))) + err := ioctl(f, ioctl_FIODNAME, uintptr(unsafe.Pointer(&fa))) if err != nil { return "", err } diff --git a/pty_freebsd.go b/pty_freebsd.go index bcd3b6f..47afcfe 100644 --- a/pty_freebsd.go +++ b/pty_freebsd.go @@ -44,8 +44,8 @@ func open() (pty, tty *os.File, err error) { return p, t, nil } -func isptmaster(fd uintptr) (bool, error) { - err := ioctl(fd, syscall.TIOCPTMASTER, 0) +func isptmaster(f *os.File) (bool, error) { + err := ioctl(f, syscall.TIOCPTMASTER, 0) return err == nil, err } @@ -55,7 +55,7 @@ var ( ) func ptsname(f *os.File) (string, error) { - master, err := isptmaster(f.Fd()) + master, err := isptmaster(f) if err != nil { return "", err } @@ -68,7 +68,7 @@ func ptsname(f *os.File) (string, error) { buf = make([]byte, n) arg = fiodgnameArg{Len: n, Buf: (*byte)(unsafe.Pointer(&buf[0]))} ) - if err := ioctl(f.Fd(), ioctlFIODGNAME, uintptr(unsafe.Pointer(&arg))); err != nil { + if err := ioctl(f, ioctlFIODGNAME, uintptr(unsafe.Pointer(&arg))); err != nil { return "", err } diff --git a/pty_linux.go b/pty_linux.go index a3b368f..e12e2c8 100644 --- a/pty_linux.go +++ b/pty_linux.go @@ -40,7 +40,7 @@ func open() (pty, tty *os.File, err error) { func ptsname(f *os.File) (string, error) { var n _C_uint - err := ioctl(f.Fd(), syscall.TIOCGPTN, uintptr(unsafe.Pointer(&n))) //nolint:gosec // Expected unsafe pointer for Syscall call. + err := ioctl(f, syscall.TIOCGPTN, uintptr(unsafe.Pointer(&n))) //nolint:gosec // Expected unsafe pointer for Syscall call. if err != nil { return "", err } @@ -50,5 +50,5 @@ func ptsname(f *os.File) (string, error) { func unlockpt(f *os.File) error { var u _C_int // use TIOCSPTLCK with a pointer to zero to clear the lock - return ioctl(f.Fd(), syscall.TIOCSPTLCK, uintptr(unsafe.Pointer(&u))) //nolint:gosec // Expected unsafe pointer for Syscall call. + return ioctl(f, syscall.TIOCSPTLCK, uintptr(unsafe.Pointer(&u))) //nolint:gosec // Expected unsafe pointer for Syscall call. } diff --git a/pty_netbsd.go b/pty_netbsd.go index 2b20d94..dd5611d 100644 --- a/pty_netbsd.go +++ b/pty_netbsd.go @@ -47,7 +47,7 @@ func ptsname(f *os.File) (string, error) { * ioctl(fd, TIOCPTSNAME, &pm) == -1 ? NULL : pm.sn; */ var ptm ptmget - if err := ioctl(f.Fd(), uintptr(ioctl_TIOCPTSNAME), uintptr(unsafe.Pointer(&ptm))); err != nil { + if err := ioctl(f, uintptr(ioctl_TIOCPTSNAME), uintptr(unsafe.Pointer(&ptm))); err != nil { return "", err } name := make([]byte, len(ptm.Sn)) @@ -65,5 +65,5 @@ func grantpt(f *os.File) error { * from grantpt(3): Calling grantpt() is equivalent to: * ioctl(fd, TIOCGRANTPT, 0); */ - return ioctl(f.Fd(), uintptr(ioctl_TIOCGRANTPT), 0) + return ioctl(f, uintptr(ioctl_TIOCGRANTPT), 0) } diff --git a/pty_openbsd.go b/pty_openbsd.go index aada5e3..337c39f 100644 --- a/pty_openbsd.go +++ b/pty_openbsd.go @@ -36,7 +36,7 @@ func open() (pty, tty *os.File, err error) { defer p.Close() var ptm ptmget - if err := ioctl(p.Fd(), uintptr(ioctl_PTMGET), uintptr(unsafe.Pointer(&ptm))); err != nil { + if err := ioctl(p, uintptr(ioctl_PTMGET), uintptr(unsafe.Pointer(&ptm))); err != nil { return nil, nil, err } diff --git a/pty_solaris.go b/pty_solaris.go index 37f933e..4e22416 100644 --- a/pty_solaris.go +++ b/pty_solaris.go @@ -65,7 +65,7 @@ func open() (pty, tty *os.File, err error) { } func ptsname(f *os.File) (string, error) { - dev, err := ptsdev(f.Fd()) + dev, err := ptsdev(f) if err != nil { return "", err } @@ -84,12 +84,12 @@ func unlockpt(f *os.File) error { icLen: 0, icDP: nil, } - return ioctl(f.Fd(), I_STR, uintptr(unsafe.Pointer(&istr))) + return ioctl(f, I_STR, uintptr(unsafe.Pointer(&istr))) } func minor(x uint64) uint64 { return x & 0377 } -func ptsdev(fd uintptr) (uint64, error) { +func ptsdev(f *os.File) (uint64, error) { istr := strioctl{ icCmd: ISPTM, icTimeout: 0, @@ -97,14 +97,33 @@ func ptsdev(fd uintptr) (uint64, error) { icDP: nil, } - if err := ioctl(fd, I_STR, uintptr(unsafe.Pointer(&istr))); err != nil { + if err := ioctl(f, I_STR, uintptr(unsafe.Pointer(&istr))); err != nil { return 0, err } - var status syscall.Stat_t - if err := syscall.Fstat(int(fd), &status); err != nil { + var errors = make(chan error, 1) + var results = make(chan uint64, 1) + defer close(errors) + defer close(results) + + var err error + var sc syscall.RawConn + sc, err = f.SyscallConn() + if err != nil { + return 0, err + } + err = sc.Control(func(fd uintptr) { + var status syscall.Stat_t + if err := syscall.Fstat(int(fd), &status); err != nil { + results <- 0 + errors <- err + } + results <- uint64(minor(status.Rdev)) + errors <- nil + }) + if err != nil { return 0, err } - return uint64(minor(status.Rdev)), nil + return <-results, <-errors } type ptOwn struct { @@ -113,7 +132,7 @@ type ptOwn struct { } func grantpt(f *os.File) error { - if _, err := ptsdev(f.Fd()); err != nil { + if _, err := ptsdev(f); err != nil { return err } pto := ptOwn{ @@ -127,7 +146,7 @@ func grantpt(f *os.File) error { icLen: int32(unsafe.Sizeof(strioctl{})), icDP: unsafe.Pointer(&pto), } - if err := ioctl(f.Fd(), I_STR, uintptr(unsafe.Pointer(&istr))); err != nil { + if err := ioctl(f, I_STR, uintptr(unsafe.Pointer(&istr))); err != nil { return errors.New("access denied") } return nil @@ -145,8 +164,8 @@ func streamsPush(f *os.File, mod string) error { // but since we are not using libc or XPG4.2, we should not be // double-pushing modules - if err := ioctl(f.Fd(), I_FIND, uintptr(unsafe.Pointer(&buf[0]))); err != nil { + if err := ioctl(f, I_FIND, uintptr(unsafe.Pointer(&buf[0]))); err != nil { return nil } - return ioctl(f.Fd(), I_PUSH, uintptr(unsafe.Pointer(&buf[0]))) + return ioctl(f, I_PUSH, uintptr(unsafe.Pointer(&buf[0]))) } diff --git a/winsize_unix.go b/winsize_unix.go index 5d99c3d..ad98c8a 100644 --- a/winsize_unix.go +++ b/winsize_unix.go @@ -20,7 +20,7 @@ type Winsize struct { // Setsize resizes t to s. func Setsize(t *os.File, ws *Winsize) error { //nolint:gosec // Expected unsafe pointer for Syscall call. - return ioctl(t.Fd(), syscall.TIOCSWINSZ, uintptr(unsafe.Pointer(ws))) + return ioctl(t, syscall.TIOCSWINSZ, uintptr(unsafe.Pointer(ws))) } // GetsizeFull returns the full terminal size description. @@ -28,7 +28,7 @@ func GetsizeFull(t *os.File) (size *Winsize, err error) { var ws Winsize //nolint:gosec // Expected unsafe pointer for Syscall call. - if err := ioctl(t.Fd(), syscall.TIOCGWINSZ, uintptr(unsafe.Pointer(&ws))); err != nil { + if err := ioctl(t, syscall.TIOCGWINSZ, uintptr(unsafe.Pointer(&ws))); err != nil { return nil, err } return &ws, nil