Skip to content

Commit

Permalink
interp: drop muesli/cancelreader for os.File.SetReadDeadline
Browse files Browse the repository at this point in the history
The docs for SetReadDeadline are very clear:

    SetReadDeadline sets the deadline for future Read calls
    and any currently-blocked Read call.

Given that it works for any currently blocked Read call as well,
we don't need all the extra machinery and code from cancelreader.
Particularly now that Reader.stdin is always a file.
  • Loading branch information
mvdan committed Dec 17, 2024
1 parent 16caae9 commit aecfbc3
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 36 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ require (
github.com/go-quicktest/qt v1.101.0
github.com/google/go-cmp v0.6.0
github.com/google/renameio/v2 v2.0.0
github.com/muesli/cancelreader v0.2.2
github.com/rogpeppe/go-internal v1.13.1
golang.org/x/sync v0.9.0
golang.org/x/sys v0.27.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA=
github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo=
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII=
Expand Down
48 changes: 15 additions & 33 deletions interp/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ import (
"context"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strconv"
"strings"
"syscall"
"time"

"github.com/muesli/cancelreader"
"golang.org/x/term"

"mvdan.cc/sh/v3/expand"
Expand Down Expand Up @@ -942,39 +941,22 @@ func (r *Runner) readLine(ctx context.Context, raw bool) ([]byte, error) {
var line []byte
esc := false

stdin := io.Reader(r.stdin)
// [cancelreader.NewReader] may fail under some circumstances, such as r.stdin being
// a regular file on Linux, in which case epoll returns an "operation not permitted" error
// given that regular files can always be read immediately. Polling them makes no sense.
// As such, if cancelreader fails, fall back to no cancellation, meaning this is best-effort.
//
// TODO: it would be nice if the cancelreader library classified errors so that we could
// safely handle "this file does not need polling" by skipping the polling as we do below
// but still fail on other errors, which may be unexpected or hide bugs.
// See the upstream issue: https://github.com/muesli/cancelreader/issues/23
if cr, err := cancelreader.NewReader(r.stdin); err == nil {
stopc := make(chan struct{})
stop := context.AfterFunc(ctx, func() {
cr.Cancel()
close(stopc)
})
defer func() {
if !stop() {
// The AfterFunc was started; wait for it to complete and close the cancel reader.
// Could put the Close in the above goroutine, but if "read" is
// immediately called again, the Close might overlap with creating a
// new cancelreader. Want this cancelreader to be completely closed
// by the time readLine returns.
<-stopc
cr.Close()
}
}()
stdin = cr
}

stopc := make(chan struct{})
stop := context.AfterFunc(ctx, func() {
r.stdin.SetReadDeadline(time.Now())
close(stopc)
})
defer func() {
if !stop() {
// The AfterFunc was started.
// Wait for it to complete, and reset the file's deadline.
<-stopc
r.stdin.SetReadDeadline(time.Time{})
}
}()
for {
var buf [1]byte
n, err := stdin.Read(buf[:])
n, err := r.stdin.Read(buf[:])
if n > 0 {
b := buf[0]
switch {
Expand Down

0 comments on commit aecfbc3

Please sign in to comment.