]> Cypherpunks repositories - gostls13.git/commitdiff
os: deflake TestPipeEOF and TestFifoEOF
authorBryan C. Mills <bcmills@google.com>
Tue, 13 Dec 2022 19:19:59 +0000 (14:19 -0500)
committerGopher Robot <gobot@golang.org>
Thu, 19 Jan 2023 20:45:34 +0000 (20:45 +0000)
- Consolidate the two test bodies as one helper function.
- Eliminate arbitrary timeout.
- Shorten arbitrary sleeps in short mode.
- Simplify goroutines.
- Mark the tests as parallel.

Fixes #36107.
Updates #24164.

Change-Id: I14fe4395963a7256cb6d2d743d348a1ade077d5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/457336
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

src/os/fifo_test.go
src/os/pipe_test.go

index de70927961314c2f4e50025482ac4c97cd243178..2f0e06bc52a626f115d4dfff9726a4501a6c5a56 100644 (file)
@@ -2,30 +2,19 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd
+//go:build darwin || dragonfly || freebsd || (linux && !android) || netbsd || openbsd
 
 package os_test
 
 import (
-       "bufio"
-       "bytes"
-       "fmt"
-       "io"
        "os"
        "path/filepath"
-       "runtime"
-       "sync"
        "syscall"
        "testing"
-       "time"
 )
 
-// Issue 24164.
 func TestFifoEOF(t *testing.T) {
-       switch runtime.GOOS {
-       case "android":
-               t.Skip("skipping on Android; mkfifo syscall not available")
-       }
+       t.Parallel()
 
        dir := t.TempDir()
        fifoName := filepath.Join(dir, "fifo")
@@ -33,71 +22,40 @@ func TestFifoEOF(t *testing.T) {
                t.Fatal(err)
        }
 
-       var wg sync.WaitGroup
-       wg.Add(1)
+       // Per https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html#tag_16_357_03:
+       //
+       // - “If O_NONBLOCK is clear, an open() for reading-only shall block the
+       //   calling thread until a thread opens the file for writing. An open() for
+       //   writing-only shall block the calling thread until a thread opens the file
+       //   for reading.”
+       //
+       // In order to unblock both open calls, we open the two ends of the FIFO
+       // simultaneously in separate goroutines.
+
+       rc := make(chan *os.File, 1)
        go func() {
-               defer wg.Done()
-
-               w, err := os.OpenFile(fifoName, os.O_WRONLY, 0)
+               r, err := os.Open(fifoName)
                if err != nil {
                        t.Error(err)
-                       return
                }
-
-               defer func() {
-                       if err := w.Close(); err != nil {
-                               t.Errorf("error closing writer: %v", err)
-                       }
-               }()
-
-               for i := 0; i < 3; i++ {
-                       time.Sleep(10 * time.Millisecond)
-                       _, err := fmt.Fprintf(w, "line %d\n", i)
-                       if err != nil {
-                               t.Errorf("error writing to fifo: %v", err)
-                               return
-                       }
-               }
-               time.Sleep(10 * time.Millisecond)
+               rc <- r
        }()
 
-       defer wg.Wait()
-
-       r, err := os.Open(fifoName)
+       w, err := os.OpenFile(fifoName, os.O_WRONLY, 0)
        if err != nil {
-               t.Fatal(err)
+               t.Error(err)
        }
 
-       done := make(chan bool)
-       go func() {
-               defer close(done)
-
-               defer func() {
-                       if err := r.Close(); err != nil {
-                               t.Errorf("error closing reader: %v", err)
-                       }
-               }()
-
-               rbuf := bufio.NewReader(r)
-               for {
-                       b, err := rbuf.ReadBytes('\n')
-                       if err == io.EOF {
-                               break
-                       }
-                       if err != nil {
-                               t.Error(err)
-                               return
-                       }
-                       t.Logf("%s\n", bytes.TrimSpace(b))
+       r := <-rc
+       if t.Failed() {
+               if r != nil {
+                       r.Close()
                }
-       }()
-
-       select {
-       case <-done:
-               // Test succeeded.
-       case <-time.After(time.Second):
-               t.Error("timed out waiting for read")
-               // Close the reader to force the read to complete.
-               r.Close()
+               if w != nil {
+                       w.Close()
+               }
+               return
        }
+
+       testPipeEOF(t, r, w)
 }
index 26565853e1ff9da08d37609b844bf3fcaa2fe701..a20a12aac40174afe5e93a3b1c66bd1adfb71c1b 100644 (file)
@@ -339,68 +339,72 @@ func testCloseWithBlockingRead(t *testing.T, r, w *os.File) {
        wg.Wait()
 }
 
-// Issue 24164, for pipes.
 func TestPipeEOF(t *testing.T) {
+       t.Parallel()
+
        r, w, err := os.Pipe()
        if err != nil {
                t.Fatal(err)
        }
 
-       var wg sync.WaitGroup
-       wg.Add(1)
-       go func() {
-               defer wg.Done()
-
-               defer func() {
-                       if err := w.Close(); err != nil {
-                               t.Errorf("error closing writer: %v", err)
-                       }
-               }()
+       testPipeEOF(t, r, w)
+}
 
-               for i := 0; i < 3; i++ {
-                       time.Sleep(10 * time.Millisecond)
-                       _, err := fmt.Fprintf(w, "line %d\n", i)
-                       if err != nil {
-                               t.Errorf("error writing to fifo: %v", err)
-                               return
-                       }
+// testPipeEOF tests that when the write side of a pipe or FIFO is closed,
+// a blocked Read call on the reader side returns io.EOF.
+//
+// This scenario previously failed to unblock the Read call on darwin.
+// (See https://go.dev/issue/24164.)
+func testPipeEOF(t *testing.T, r io.ReadCloser, w io.WriteCloser) {
+       // parkDelay is an arbitrary delay we wait for a pipe-reader goroutine to park
+       // before issuing the corresponding write. The test should pass no matter what
+       // delay we use, but with a longer delay is has a higher chance of detecting
+       // poller bugs.
+       parkDelay := 10 * time.Millisecond
+       if testing.Short() {
+               parkDelay = 100 * time.Microsecond
+       }
+       writerDone := make(chan struct{})
+       defer func() {
+               if err := r.Close(); err != nil {
+                       t.Errorf("error closing reader: %v", err)
                }
-               time.Sleep(10 * time.Millisecond)
+               <-writerDone
        }()
 
-       defer wg.Wait()
-
-       done := make(chan bool)
+       write := make(chan int, 1)
        go func() {
-               defer close(done)
-
-               defer func() {
-                       if err := r.Close(); err != nil {
-                               t.Errorf("error closing reader: %v", err)
-                       }
-               }()
+               defer close(writerDone)
 
-               rbuf := bufio.NewReader(r)
-               for {
-                       b, err := rbuf.ReadBytes('\n')
-                       if err == io.EOF {
-                               break
-                       }
+               for i := range write {
+                       time.Sleep(parkDelay)
+                       _, err := fmt.Fprintf(w, "line %d\n", i)
                        if err != nil {
-                               t.Error(err)
+                               t.Errorf("error writing to fifo: %v", err)
                                return
                        }
-                       t.Logf("%s\n", bytes.TrimSpace(b))
+               }
+
+               time.Sleep(parkDelay)
+               if err := w.Close(); err != nil {
+                       t.Errorf("error closing writer: %v", err)
                }
        }()
 
-       select {
-       case <-done:
-               // Test succeeded.
-       case <-time.After(time.Second):
-               t.Error("timed out waiting for read")
-               // Close the reader to force the read to complete.
-               r.Close()
+       rbuf := bufio.NewReader(r)
+       for i := 0; i < 3; i++ {
+               write <- i
+               b, err := rbuf.ReadBytes('\n')
+               if err != nil {
+                       t.Fatal(err)
+               }
+               t.Logf("%s\n", bytes.TrimSpace(b))
+       }
+
+       close(write)
+       b, err := rbuf.ReadBytes('\n')
+       if err != io.EOF || len(b) != 0 {
+               t.Errorf(`ReadBytes: %q, %v; want "", io.EOF`, b, err)
        }
 }