From f4fed36bfda1a98efdc15cbd44e1d5b6137c9a65 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Mon, 15 Jul 2024 20:30:47 +0200 Subject: [PATCH] Fix a hang Before this change, if select() returned EINTR we would hang. This change: * Makes us ignore EINTRs and try again * Makes sure to not hang, even if the twin main loop would fail for some other reason. --- build.sh | 2 +- twin/screen-setup-windows.go | 4 ++++ twin/screen-setup.go | 25 +++++++++++++++++++++++-- twin/screen.go | 7 +++++++ 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/build.sh b/build.sh index 1f37224..0296c66 100755 --- a/build.sh +++ b/build.sh @@ -28,4 +28,4 @@ fi go build -trimpath -ldflags="-s -w -X main.versionString=${VERSION}" -o "${BINARY}" # Alternative build line, if you want to attach to the running process in the Go debugger: -# go build -ldflags="-X main.versionString=${VERSION}" -gcflags='-N -l' -o "${BINARY}" +# go build -ldflags="-X main.versionString=${VERSION}" -gcflags="all=-N -l" -o "${BINARY}" diff --git a/twin/screen-setup-windows.go b/twin/screen-setup-windows.go index 0bdcd4e..a22a998 100644 --- a/twin/screen-setup-windows.go +++ b/twin/screen-setup-windows.go @@ -55,6 +55,10 @@ func (r *interruptableReaderImpl) Interrupt() { r.shutdownRequested.Store(true) } +func (r *interruptableReaderImpl) Close() error { + return nil +} + func newInterruptableReader(base *os.File) (interruptableReader, error) { return &interruptableReaderImpl{base: base}, nil } diff --git a/twin/screen-setup.go b/twin/screen-setup.go index f1f1741..8ca1533 100644 --- a/twin/screen-setup.go +++ b/twin/screen-setup.go @@ -24,6 +24,19 @@ type interruptableReaderImpl struct { } func (r *interruptableReaderImpl) Read(p []byte) (n int, err error) { + for { + n, err = r.read(p) + if err == syscall.EINTR { + // Not really a problem, we can get this on window resizes for + // example, just try again. + continue + } + + return + } +} + +func (r *interruptableReaderImpl) read(p []byte) (n int, err error) { // "This argument should be set to the highest-numbered file descriptor in // any of the three sets, plus 1. The indicated file descriptors in each set // are checked, up to this limit" @@ -55,7 +68,7 @@ func (r *interruptableReaderImpl) Read(p []byte) (n int, err error) { err = io.EOF // Let Interrupt() know we're done - r.interruptionComplete <- struct{}{} + r.Close() return } @@ -77,10 +90,18 @@ func (r *interruptableReaderImpl) Interrupt() { <-r.interruptionComplete } +func (r *interruptableReaderImpl) Close() error { + select { + case r.interruptionComplete <- struct{}{}: + default: + } + return nil +} + func newInterruptableReader(base *os.File) (interruptableReader, error) { reader := interruptableReaderImpl{ base: base, - interruptionComplete: make(chan struct{}), + interruptionComplete: make(chan struct{}, 1), } pr, pw, err := os.Pipe() if err != nil { diff --git a/twin/screen.go b/twin/screen.go index 27d00a5..92c6d52 100644 --- a/twin/screen.go +++ b/twin/screen.go @@ -74,6 +74,12 @@ type interruptableReader interface { // Interrupt unblocks the read call, either now or eventually. Interrupt() + + // Close() should be called after you are done with the interruptableReader. + // + // It will not close the underlying reader, but it will prevent Interrupt() + // from hanging if called after a failure in the screen mainLoop(). + Close() error } type UnixScreen struct { @@ -366,6 +372,7 @@ func (screen *UnixScreen) ShowCursorAt(column int, row int) { func (screen *UnixScreen) mainLoop() { defer func() { + screen.ttyInReader.Close() log.Debug("Twin screen main loop done") }()