Fix closing IOStreams.Out after finishing writing to the pager (#6210)

pkg/iostreams/iostreams.go

@@ -214,9 +214,9 @@ func (s *IOStreams) StartPager() error {
 	if err != nil {
 		return err
 	}
-	s.Out = &fdWriter{
-		fd:     s.Out.Fd(),
-		Writer: &pagerWriter{pagedOut},
+	s.Out = &fdWriteCloser{
+		fd:          s.Out.Fd(),
+		WriteCloser: &pagerWriter{pagedOut},
 	}
 	err = pagerCmd.Start()
 	if err != nil {
@@ -231,6 +231,7 @@ func (s *IOStreams) StopPager() {
 		return
 	}
 
+	// if a pager was started, we're guaranteed to have a WriteCloser
 	_ = s.Out.(io.WriteCloser).Close()
 	_, _ = s.pagerProcess.Wait()
 	s.pagerProcess = nil
@@ -521,6 +522,16 @@ func (w *fdWriter) Fd() uintptr {
 	return w.fd
 }
 
+// fdWriteCloser represents a wrapped stdout Writer that preserves the original file descriptor
+type fdWriteCloser struct {
+	io.WriteCloser
+	fd uintptr
+}
+
+func (w *fdWriteCloser) Fd() uintptr {
+	return w.fd
+}
+
 // fdWriter represents a wrapped stdin ReadCloser that preserves the original file descriptor
 type fdReader struct {
 	io.ReadCloser

pkg/iostreams/iostreams_test.go

@@ -1,9 +1,11 @@
 package iostreams
 
 import (
+	"bufio"
 	"errors"
 	"fmt"
 	"io"
+	"os"
 	"testing"
 )
 
@@ -89,3 +91,40 @@ func TestStopAlternateScreenBuffer(t *testing.T) {
 		t.Errorf("after IOStreams.StopAlternateScreenBuffer() got %q, want %q", got, want)
 	}
 }
+
+func TestIOStreams_pager(t *testing.T) {
+	t.Skip("TODO: fix this test in race detection mode")
+	ios, _, stdout, _ := Test()
+	ios.SetStdoutTTY(true)
+	ios.SetPager(fmt.Sprintf("%s -test.run=TestHelperProcess --", os.Args[0]))
+	t.Setenv("GH_WANT_HELPER_PROCESS", "1")
+	if err := ios.StartPager(); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := fmt.Fprintln(ios.Out, "line1"); err != nil {
+		t.Errorf("error writing line 1: %v", err)
+	}
+	if _, err := fmt.Fprintln(ios.Out, "line2"); err != nil {
+		t.Errorf("error writing line 2: %v", err)
+	}
+	ios.StopPager()
+	wants := "pager: line1\npager: line2\n"
+	if got := stdout.String(); got != wants {
+		t.Errorf("expected %q, got %q", wants, got)
+	}
+}
+
+func TestHelperProcess(t *testing.T) {
+	if os.Getenv("GH_WANT_HELPER_PROCESS") != "1" {
+		return
+	}
+	scanner := bufio.NewScanner(os.Stdin)
+	for scanner.Scan() {
+		fmt.Printf("pager: %s\n", scanner.Text())
+	}
+	if err := scanner.Err(); err != nil {
+		fmt.Fprintf(os.Stderr, "error reading stdin: %v", err)
+		os.Exit(1)
+	}
+	os.Exit(0)
+}