From 6e82febaf0ca737e82cc3f53de7245101821821c Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" <bcmills@google.com>
Date: Wed, 25 Jan 2023 11:53:03 -0500
Subject: [PATCH] os: eliminate arbitrary timeout in testClosewithBlockingRead

The 1-second timeout on execution of this test is empirically too
short on some platforms. Rather than trying to tune the timeout, allow
the test to time out on its own (and dump goroutines) if it deadlocks.

Fixes #57993.
Fixes #57994.

Change-Id: I69ee86c75034469e4b4cd391b8dc5616b93468b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/463180
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
---
 src/os/pipe_test.go | 72 ++++++++++++++-------------------------------
 1 file changed, 22 insertions(+), 50 deletions(-)

diff --git a/src/os/pipe_test.go b/src/os/pipe_test.go
index 27a96ad586..d0a4c65de2 100644
--- a/src/os/pipe_test.go
+++ b/src/os/pipe_test.go
@@ -299,68 +299,40 @@ func TestCloseWithBlockingReadByFd(t *testing.T) {
 
 // Test that we don't let a blocking read prevent a close.
 func testCloseWithBlockingRead(t *testing.T, r, w *os.File) {
-	defer r.Close()
-	defer w.Close()
-
-	c1, c2 := make(chan bool), make(chan bool)
-	var wg sync.WaitGroup
-
-	wg.Add(1)
-	go func(c chan bool) {
-		defer wg.Done()
-		// Give the other goroutine a chance to enter the Read
-		// or Write call. This is sloppy but the test will
-		// pass even if we close before the read/write.
-		time.Sleep(20 * time.Millisecond)
-
-		if err := r.Close(); err != nil {
-			t.Error(err)
-		}
-		close(c)
-	}(c1)
-
-	wg.Add(1)
-	go func(c chan bool) {
-		defer wg.Done()
+	var (
+		enteringRead = make(chan struct{})
+		done         = make(chan struct{})
+	)
+	go func() {
 		var b [1]byte
+		close(enteringRead)
 		_, err := r.Read(b[:])
-		close(c)
 		if err == nil {
 			t.Error("I/O on closed pipe unexpectedly succeeded")
 		}
+
 		if pe, ok := err.(*fs.PathError); ok {
 			err = pe.Err
 		}
 		if err != io.EOF && err != fs.ErrClosed {
 			t.Errorf("got %v, expected EOF or closed", err)
 		}
-	}(c2)
-
-	for c1 != nil || c2 != nil {
-		select {
-		case <-c1:
-			c1 = nil
-			// r.Close has completed, but the blocking Read
-			// is hanging. Close the writer to unblock it.
-			w.Close()
-		case <-c2:
-			c2 = nil
-		case <-time.After(1 * time.Second):
-			switch {
-			case c1 != nil && c2 != nil:
-				t.Error("timed out waiting for Read and Close")
-				w.Close()
-			case c1 != nil:
-				t.Error("timed out waiting for Close")
-			case c2 != nil:
-				t.Error("timed out waiting for Read")
-			default:
-				t.Error("impossible case")
-			}
-		}
-	}
+		close(done)
+	}()
 
-	wg.Wait()
+	// Give the goroutine a chance to enter the Read
+	// or Write call. This is sloppy but the test will
+	// pass even if we close before the read/write.
+	<-enteringRead
+	time.Sleep(20 * time.Millisecond)
+
+	if err := r.Close(); err != nil {
+		t.Error(err)
+	}
+	// r.Close has completed, but since we assume r is in blocking mode that
+	// probably didn't unblock the call to r.Read. Close w to unblock it.
+	w.Close()
+	<-done
 }
 
 func TestPipeEOF(t *testing.T) {
-- 
2.48.1