Skip to content

Commit

Permalink
Fix a hang
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
walles committed Jul 15, 2024
1 parent 2781774 commit f4fed36
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 3 deletions.
2 changes: 1 addition & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
4 changes: 4 additions & 0 deletions twin/screen-setup-windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
25 changes: 23 additions & 2 deletions twin/screen-setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions twin/screen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
}()

Expand Down

0 comments on commit f4fed36

Please sign in to comment.