Try fixing CI

.github/workflows/linux-ci.yml

@@ -22,7 +22,7 @@ jobs:
       # commandline below:
       # https://github.com/golangci/golangci-lint/releases/latest
       - name: Install golangci-lint
-        run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b "$(go env GOPATH)"/bin v2.1.2
+        run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b "$(go env GOPATH)"/bin v2.11.2
 
       - run: go build ./...
       - run: ./test.sh

twin/interruptablereader.go

@@ -19,7 +19,8 @@ type interruptableReader struct {
 	pauseOrRead semaphore.Weighted
 }
 
-const interruptableReaderPollInterval = 100 * time.Millisecond
+// Basically how long we wait between interrupt checks
+const interruptableReaderMaxWait = 100 * time.Millisecond
 
 func newInterruptableReader(base *os.File) interruptableReader {
 	return interruptableReader{
@@ -55,7 +56,7 @@ func (r *interruptableReader) Read(p []byte) (n int, err error) {
 			return 0, io.EOF
 		}
 
-		ready, waitErr := r.waitForReadReady()
+		ready, waitErr := r.waitForReadReady(interruptableReaderMaxWait)
 		if waitErr != nil {
 			return 0, waitErr
 		}

twin/screen-setup-windows.go

@@ -8,13 +8,68 @@ import (
 	"runtime/debug"
 	"syscall"
 	"time"
+	"unsafe"
 
 	"golang.org/x/sys/windows"
 	"golang.org/x/term"
 )
 
-func (r *interruptableReader) waitForReadReady() (ready bool, err error) {
-	timeoutMillis := uint32(interruptableReaderPollInterval.Milliseconds())
+var peekNamedPipe = windows.NewLazySystemDLL("kernel32.dll").NewProc("PeekNamedPipe")
+
+func waitForPipeReadReady(handle windows.Handle) (ready bool, err error) {
+	var bytesAvailable uint32
+	result, _, callErr := peekNamedPipe.Call(
+		uintptr(handle),
+		0,
+		0,
+		0,
+		uintptr(unsafe.Pointer(&bytesAvailable)),
+		0,
+	)
+	if result != 0 {
+		return bytesAvailable > 0, nil
+	}
+
+	if callErr == windows.ERROR_BROKEN_PIPE {
+		// Writer closed: let a real Read() return EOF.
+		return true, nil
+	}
+
+	if callErr == windows.ERROR_NO_DATA {
+		// Pipe has no data right now.
+		return false, nil
+	}
+
+	if callErr == windows.ERROR_HANDLE_EOF {
+		return true, nil
+	}
+
+	return false, fmt.Errorf("PeekNamedPipe failed: %w", callErr)
+}
+
+func (r *interruptableReader) waitForReadReady(timeout time.Duration) (ready bool, err error) {
+	fileType, err := windows.GetFileType(windows.Handle(r.base.Fd()))
+	if err != nil {
+		return false, err
+	}
+
+	if fileType == windows.FILE_TYPE_PIPE {
+		ready, err = waitForPipeReadReady(windows.Handle(r.base.Fd()))
+		if ready || err != nil {
+			return
+		}
+
+		time.Sleep(timeout / 2)
+		ready, err = waitForPipeReadReady(windows.Handle(r.base.Fd()))
+		if ready || err != nil {
+			return
+		}
+
+		time.Sleep(timeout / 2)
+		return
+	}
+
+	timeoutMillis := uint32(timeout.Milliseconds())
 	if timeoutMillis == 0 {
 		timeoutMillis = 1
 	}

twin/screen-setup.go

@@ -8,12 +8,13 @@ import (
 	"os/signal"
 	"runtime/debug"
 	"syscall"
+	"time"
 
 	"golang.org/x/sys/unix"
 	"golang.org/x/term"
 )
 
-func (r *interruptableReader) waitForReadReady() (ready bool, err error) {
+func (r *interruptableReader) waitForReadReady(timeout time.Duration) (ready bool, 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"
@@ -22,9 +23,9 @@ func (r *interruptableReader) waitForReadReady() (ready bool, err error) {
 	nfds := r.base.Fd()
 	readFds := unix.FdSet{}
 	readFds.Set(int(r.base.Fd()))
-	timeout := unix.NsecToTimeval(interruptableReaderPollInterval.Nanoseconds())
+	selectTimeout := unix.NsecToTimeval(timeout.Nanoseconds())
 
-	_, err = unix.Select(int(nfds)+1, &readFds, nil, nil, &timeout)
+	_, err = unix.Select(int(nfds)+1, &readFds, nil, nil, &selectTimeout)
 	if err == syscall.EINTR {
 		// Not really a problem, we can get this on window resizes for example
 		return false, nil

twin/screen_test.go

@@ -378,3 +378,59 @@ func TestInterruptableReader_justRead(t *testing.T) {
 	assert.Equal(t, buffer[0], byte(42))
 	assert.Equal(t, len(buffer), 7)
 }
+
+func TestInterruptableReader_waitForReadReadyPipe(t *testing.T) {
+	// Make a pipe to read from and write to
+	pipeReader, pipeWriter, err := os.Pipe()
+	assert.NilError(t, err)
+
+	t.Cleanup(func() {
+		_ = pipeReader.Close()
+		_ = pipeWriter.Close()
+	})
+
+	// Make an interruptable reader
+	testMe := newInterruptableReader(pipeReader)
+
+	// With no data available we should wait a bit, then report not ready.
+	t0 := time.Now()
+	ready, err := testMe.waitForReadReady(time.Millisecond * 100)
+	duration := time.Since(t0)
+	assert.NilError(t, err)
+	assert.Equal(t, ready, false)
+	assert.Assert(t, duration > time.Millisecond*100)
+
+	// After writing, the pipe should become ready.
+	n, err := pipeWriter.Write([]byte{42})
+	assert.NilError(t, err)
+	assert.Equal(t, n, 1)
+
+	// With data available we should report ready immediately
+	ready, err = testMe.waitForReadReady(time.Hour)
+	assert.NilError(t, err)
+	assert.Equal(t, ready, true)
+}
+
+// On Unix, files are always ready (to return EOF if nothing else), but on
+// Windows they are non-ready if they have no data. So we just verify the
+// have-data case here, and let the no-data case be whatever.
+func TestInterruptableReader_waitForReadReadyFile(t *testing.T) {
+	tempFile, err := os.CreateTemp("", "moor-wait-for-read-ready-*.txt")
+	assert.NilError(t, err)
+
+	t.Cleanup(func() {
+		_ = tempFile.Close()
+		_ = os.Remove(tempFile.Name())
+	})
+
+	// Put something in the file
+	n, err := tempFile.Write([]byte("x"))
+	assert.NilError(t, err)
+	assert.Equal(t, n, 1)
+
+	// Expect read-ready immediately
+	testMe := newInterruptableReader(tempFile)
+	ready, err := testMe.waitForReadReady(time.Hour)
+	assert.NilError(t, err)
+	assert.Equal(t, ready, true)
+}