Skip to content

Commit

Permalink
Work around pty closing bug on windows
Browse files Browse the repository at this point in the history
  • Loading branch information
Naatan committed Sep 1, 2023
1 parent b353f0f commit d5396a3
Show file tree
Hide file tree
Showing 599 changed files with 225,718 additions and 880 deletions.
2 changes: 1 addition & 1 deletion expect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
func Test_Expect(t *testing.T) {
tt := newTermTest(t, exec.Command("bash", "-c", "echo HELLO"), true)
tt.Expect("HELLO")
tt.ExpectExitCode(0)
tt.ExpectExitCode(0, OptExpectTimeout(time.Hour))
}

func Test_Expect_Cmd(t *testing.T) {
Expand Down
12 changes: 10 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,22 @@ go 1.18
require (
github.com/ActiveState/pty v0.0.0-20230628221854-6fb90eb08a14
github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02
github.com/stretchr/testify v1.8.0
github.com/shirou/gopsutil/v3 v3.23.8
github.com/stretchr/testify v1.8.4
go.uber.org/goleak v1.2.0
golang.org/x/sys v0.0.0-20220721230656-c6bc011c0c49
golang.org/x/sys v0.11.0
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/go-ole/go-ole v1.2.6 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
github.com/shoenig/go-m1cpu v0.1.6 // indirect
github.com/tklauser/go-sysconf v0.3.12 // indirect
github.com/tklauser/numcpus v0.6.1 // indirect
github.com/yusufpapurcu/wmi v1.2.3 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
32 changes: 30 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,52 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY=
github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0=
github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02 h1:AgcIVYPa6XJnU3phs104wLj8l5GEththEw6+F79YsIY=
github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02/go.mod h1:Q48J4R4DvxnHolD5P8pOtXigYlRuPLGl6moFx3ulM68=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4=
github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c h1:ncq/mPwQF4JjgDlrVEn3C11VoGHZN7m8qihwgMEtzYw=
github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE=
github.com/shirou/gopsutil/v3 v3.23.8 h1:xnATPiybo6GgdRoC4YoGnxXZFRc3dqQTGi73oLvvBrE=
github.com/shirou/gopsutil/v3 v3.23.8/go.mod h1:7hmCaBn+2ZwaZOr6jmPBZDfawwMGuo1id3C6aM8EDqQ=
github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM=
github.com/shoenig/go-m1cpu v0.1.6/go.mod h1:1JJMcUBvfNwpq05QDQVAnx3gUHr9IYF7GNg9SUEw2VQ=
github.com/shoenig/test v0.6.4 h1:kVTaSd7WLz5WZ2IaoM0RSzRsUD+m8wRR+5qvntpn4LU=
github.com/shoenig/test v0.6.4/go.mod h1:byHiCGXqrVaflBLAMq/srcZIHynQPQgeyvkvXnjqq0k=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
github.com/tklauser/go-sysconf v0.3.12 h1:0QaGUFOdQaIVdPgfITYzaTegZvdCjmYO52cSFAEVmqU=
github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI=
github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk=
github.com/tklauser/numcpus v0.6.1/go.mod h1:1XfjsgE2zo8GVw7POkMbHENHzVg3GzmoZ9fESEdAacY=
github.com/yusufpapurcu/wmi v1.2.3 h1:E1ctvB7uKFMOJw3fdOW32DwGE9I7t++CRUEMKvFoFiw=
github.com/yusufpapurcu/wmi v1.2.3/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0=
go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk=
go.uber.org/goleak v1.2.0/go.mod h1:XJYK+MuIchqpmGmUSAzotztawfKvYLUIgg7guXrwVUo=
golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs=
golang.org/x/sys v0.0.0-20220721230656-c6bc011c0c49 h1:TMjZDarEwf621XDryfitp/8awEhiZNiwgphKlTMGRIg=
golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20220721230656-c6bc011c0c49/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM=
golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
Expand Down
5 changes: 5 additions & 0 deletions helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -97,3 +98,7 @@ type BlackholeWriter struct {
func (BlackholeWriter) Write(p []byte) (int, error) {
return len(p), nil
}

func toPosixPath(p string) string {
return regexp.MustCompile(`^([A-Za-z])\:`).ReplaceAllString(filepath.ToSlash(p), "/$1")
}
2 changes: 1 addition & 1 deletion outputproducer.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (o *outputProducer) flushConsumers() error {
o.snapshotPos += endPos

// Drop consumer
o.opts.Logger.Printf("dropping consumer %d out of %d", n, len(o.consumers))
o.opts.Logger.Printf("dropping consumer %d out of %d", n+1, len(o.consumers))
o.consumers = append(o.consumers[:n], o.consumers[n+1:]...)
n--
}
Expand Down
33 changes: 0 additions & 33 deletions termtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,39 +239,6 @@ func (tt *TermTest) Wait(timeout time.Duration) error {
}
}

func (tt *TermTest) WaitIndefinitely() error {
tt.opts.Logger.Println("WaitIndefinitely called")
defer tt.opts.Logger.Println("WaitIndefinitely closed")

// On windows there is a race condition where ClosePseudoConsole will hang if we call it around the same
// time as the parent process exits.
// This is not a clean solution, as there's no guarantee that 100 milliseconds will be sufficient. But in
// my tests it has been, and I can't afford to keep digging on this.
if runtime.GOOS == "windows" {
time.Sleep(time.Millisecond * 100)
}

// listenError will be written to when the process exits, and this is the only reasonable place for us to
// catch listener errors
listenError := <-tt.listenError

// Clean up pty
tt.opts.Logger.Println("Closing pty")
if err := tt.ptmx.Close(); err != nil {
if syscallErrorCode(err) == 0 {
tt.opts.Logger.Println("Ignoring 'The operation completed successfully' error")
} else if errors.Is(err, ERR_ACCESS_DENIED) {
// Ignore access denied error - means process has already finished
tt.opts.Logger.Println("Ignoring access denied error")
} else {
return errors.Join(listenError, fmt.Errorf("failed to close pty: %w", err))
}
}
tt.opts.Logger.Println("Closed pty")

return listenError
}

// Cmd returns the underlying command
func (tt *TermTest) Cmd() *exec.Cmd {
return tt.cmd
Expand Down
24 changes: 24 additions & 0 deletions termtest_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,27 @@ package termtest
func syscallErrorCode(err error) int {
return -1
}

func (tt *TermTest) WaitIndefinitely() error {
tt.opts.Logger.Println("WaitIndefinitely called")
defer tt.opts.Logger.Println("WaitIndefinitely closed")

// Wait for listener to exit
listenError := <-tt.listenError

// Clean up pty
tt.opts.Logger.Println("Closing pty")
if err := tt.ptmx.Close(); err != nil {
if syscallErrorCode(err) == 0 {
tt.opts.Logger.Println("Ignoring 'The operation completed successfully' error")
} else if errors.Is(err, ERR_ACCESS_DENIED) {

Check failure on line 22 in termtest_other.go

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

undefined: errors

Check failure on line 22 in termtest_other.go

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

undefined: errors
// Ignore access denied error - means process has already finished
tt.opts.Logger.Println("Ignoring access denied error")
} else {
return errors.Join(listenError, fmt.Errorf("failed to close pty: %w", err))

Check failure on line 26 in termtest_other.go

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

undefined: errors

Check failure on line 26 in termtest_other.go

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

undefined: fmt

Check failure on line 26 in termtest_other.go

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

undefined: errors

Check failure on line 26 in termtest_other.go

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

undefined: fmt
}
}
tt.opts.Logger.Println("Closed pty")

return listenError
}
16 changes: 13 additions & 3 deletions termtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func Test_PendingOutput_Output_Snapshot(t *testing.T) {
}

func Test_ColSize(t *testing.T) {
size := 1000
size := 100
shell := "bash"
if runtime.GOOS == "windows" {
shell = "cmd.exe"
Expand All @@ -212,7 +212,12 @@ func Test_Multiline_Sanitized(t *testing.T) {
f.WriteString("foo\r\nbar")
f.Close()

tt.SendLine("cat " + f.Name())
fpath := f.Name()
if runtime.GOOS == "windows" {
fpath = toPosixPath(fpath)
}

tt.SendLine("cat " + fpath)
tt.Expect("foo\nbar")
tt.SendLine("exit")
tt.ExpectExitCode(0)
Expand All @@ -227,7 +232,12 @@ func Test_Multiline_Normalized(t *testing.T) {
f.WriteString("foo\r\nbar")
f.Close()

tt.SendLine("cat " + f.Name())
fpath := f.Name()
if runtime.GOOS == "windows" {
fpath = toPosixPath(fpath)
}

tt.SendLine("cat " + fpath)
tt.Expect("foo\nbar")
tt.SendLine("echo -e \"foo\r\nbar\" | tr -d -c \"\\r\" | wc -c")
tt.Expect("0") // Should be zero occurrences of \r
Expand Down
55 changes: 54 additions & 1 deletion termtest_windows.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,63 @@
package termtest

import "syscall"
import (
"errors"
"fmt"
"syscall"
"time"

gopsutil "github.com/shirou/gopsutil/v3/process"
)

func syscallErrorCode(err error) int {
if errv, ok := err.(syscall.Errno); ok {
return int(errv)
}
return 0
}

// WaitIndefinitely on Windows has to work around a Windows PTY bug where the PTY will NEVER exit by itself:
// https://github.com/photostorm/pty/issues/3
// Instead we wait for the process itself to exit, and after a grace period will shut down the pty.
func (tt *TermTest) WaitIndefinitely() error {
tt.opts.Logger.Println("WaitIndefinitely called")
defer tt.opts.Logger.Println("WaitIndefinitely closed")

var procErr error

tt.opts.Logger.Printf("Waiting for PID %d to exit\n", tt.Cmd().Process.Pid)
for {
// There is a race condition here; which is that the pty could still be processing the last of the output
// when the process exits. This sleep tries to work around this, but on slow hosts this may not be sufficient.
// This also gives some time in between process lookups
time.Sleep(100 * time.Millisecond)

// For some reason os.Process will always return a process even when the process has exited.
// According to the docs this shouldn't happen, but here we are.
// Using gopsutil seems to correctly identify the (not) running process.
exists, err := gopsutil.PidExists(int32(tt.Cmd().Process.Pid))
if err != nil {
return fmt.Errorf("could not find process: %d: %w", tt.Cmd().Process.Pid, err)
}
if !exists {
break
}
}

// Clean up pty
tt.opts.Logger.Println("Closing pty")
if err := tt.ptmx.Close(); err != nil {
if syscallErrorCode(err) == 0 {
tt.opts.Logger.Println("Ignoring 'The operation completed successfully' error")
} else if errors.Is(err, ERR_ACCESS_DENIED) {
// Ignore access denied error - means process has already finished
tt.opts.Logger.Println("Ignoring access denied error")
} else {
return errors.Join(procErr, fmt.Errorf("failed to close pty: %w", err))
}
}
tt.opts.Logger.Println("Closed pty")

// Now that the ptmx was closed the listener should also shut down
return errors.Join(procErr, <-tt.listenError)
}
8 changes: 8 additions & 0 deletions vendor/github.com/go-ole/go-ole/.travis.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 49 additions & 0 deletions vendor/github.com/go-ole/go-ole/ChangeLog.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions vendor/github.com/go-ole/go-ole/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit d5396a3

Please sign in to comment.