Skip to content

Commit

Permalink
test: Address a race in test with initializatoin order changes.
Browse files Browse the repository at this point in the history
Per the comment in `pipestream_unix_test.go` the write end needs to exist to
stop the read end thinking it's EOF already.

Do we need to introduce liveness hacks to decide if EOF is meaningful?
  • Loading branch information
jaqx0r committed Jul 12, 2024
1 parent 08c5cd2 commit 22703eb
Showing 1 changed file with 16 additions and 8 deletions.
24 changes: 16 additions & 8 deletions internal/tailer/logstream/pipestream_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ import (
"golang.org/x/sys/unix"
)

// In pipestream tests we must create the write end of the pipe before opening
// the logstream. If the logstream is created first, then there is an
// opportuity for a race if the Read gets scheduled before the write end has
// opened, because then https://github.com/golang/go/issues/44176 occurs -- the
// call returns EOF signalling the end of input. This might actually be
// desirable behaviour because `pipe(7)` says that if all writers are closed, a
// read returns EOF.

func TestPipeStreamReadCompletedBecauseClosed(t *testing.T) {
testutil.TimeoutTest(1*time.Second, func(t *testing.T) { //nolint:thelper
var wg sync.WaitGroup
Expand All @@ -32,10 +40,6 @@ func TestPipeStreamReadCompletedBecauseClosed(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
waker := waker.NewTestAlways()

// In this and the following test, open RDWR so as to not block this thread
// from proceeding. If we open the logstream first, there is a race before
// the write end opens that can sometimes lead to the logstream reading an
// EOF (because the write end is not yet open) and the test fails.
f, err := os.OpenFile(name, os.O_RDWR, os.ModeNamedPipe)
testutil.FatalIfErr(t, err)

Expand All @@ -49,6 +53,9 @@ func TestPipeStreamReadCompletedBecauseClosed(t *testing.T) {

testutil.WriteString(t, f, "1\n")

// Park this thread and hope the logstream gets a chance to read it.
time.Sleep(10 * time.Millisecond)

// Pipes need to be closed to signal to the pipeStream to finish up.
testutil.FatalIfErr(t, f.Close())

Expand Down Expand Up @@ -111,6 +118,9 @@ func TestPipeStreamReadURL(t *testing.T) {
defer cancel()
waker := waker.NewTestAlways()

f, err := os.OpenFile(name, os.O_RDWR, os.ModeNamedPipe)
testutil.FatalIfErr(t, err)

ps, err := logstream.New(ctx, &wg, waker, "file://"+name, logstream.OneShotDisabled)
testutil.FatalIfErr(t, err)

Expand All @@ -120,15 +130,13 @@ func TestPipeStreamReadURL(t *testing.T) {
}
checkLineDiff := testutil.ExpectLinesReceivedNoDiff(t, expected, ps.Lines())

f, err := os.OpenFile(name, os.O_WRONLY, os.ModeNamedPipe)
testutil.FatalIfErr(t, err)
testutil.WriteString(t, f, "1\n")

testutil.WriteString(t, f, "2\n")

// Give the stream a chance to wake and read
time.Sleep(10 * time.Millisecond)

testutil.WriteString(t, f, "2\n")

// Pipes need to be closed to signal to the pipeStream to finish up.
testutil.FatalIfErr(t, f.Close())

Expand Down

0 comments on commit 22703eb

Please sign in to comment.