Skip to content

Commit

Permalink
Merge pull request #14376 from biosvs/fix-expect-package
Browse files Browse the repository at this point in the history
Fixed infinite loop in ExpectProcess.ExpectFunc
  • Loading branch information
ahrtr authored Aug 24, 2022
2 parents e363480 + 71c5360 commit 82bf79d
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 8 deletions.
19 changes: 16 additions & 3 deletions pkg/expect/expect.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package expect

import (
"bufio"
"context"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -103,7 +104,7 @@ func (ep *ExpectProcess) read() {
}

// ExpectFunc returns the first line satisfying the function f.
func (ep *ExpectProcess) ExpectFunc(f func(string) bool) (string, error) {
func (ep *ExpectProcess) ExpectFunc(ctx context.Context, f func(string) bool) (string, error) {
i := 0

for {
Expand All @@ -121,7 +122,13 @@ func (ep *ExpectProcess) ExpectFunc(f func(string) bool) (string, error) {
break
}
ep.mu.Unlock()
time.Sleep(time.Millisecond * 10)

select {
case <-ctx.Done():
return "", fmt.Errorf("failed to find match string: %w", ctx.Err())
case <-time.After(time.Millisecond * 10):
// continue loop
}
}
ep.mu.Lock()
lastLinesIndex := len(ep.lines) - DEBUG_LINES_TAIL
Expand All @@ -135,9 +142,15 @@ func (ep *ExpectProcess) ExpectFunc(f func(string) bool) (string, error) {
ep.err, lastLines)
}

// ExpectWithContext returns the first line containing the given string.
func (ep *ExpectProcess) ExpectWithContext(ctx context.Context, s string) (string, error) {
return ep.ExpectFunc(ctx, func(txt string) bool { return strings.Contains(txt, s) })
}

// Expect returns the first line containing the given string.
// Deprecated: please use ExpectWithContext instead.
func (ep *ExpectProcess) Expect(s string) (string, error) {
return ep.ExpectFunc(func(txt string) bool { return strings.Contains(txt, s) })
return ep.ExpectWithContext(context.Background(), s)
}

// LineCount returns the number of recorded lines since
Expand Down
32 changes: 31 additions & 1 deletion pkg/expect/expect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@
package expect

import (
"context"
"os"
"testing"
"time"

"github.com/stretchr/testify/require"
)

func TestExpectFunc(t *testing.T) {
Expand All @@ -28,7 +31,7 @@ func TestExpectFunc(t *testing.T) {
t.Fatal(err)
}
wstr := "hello world\r\n"
l, eerr := ep.ExpectFunc(func(a string) bool { return len(a) > 10 })
l, eerr := ep.ExpectFunc(context.Background(), func(a string) bool { return len(a) > 10 })
if eerr != nil {
t.Fatal(eerr)
}
Expand All @@ -40,6 +43,33 @@ func TestExpectFunc(t *testing.T) {
}
}

func TestExpectFuncTimeout(t *testing.T) {
ep, err := NewExpect("tail", "-f", "/dev/null")
if err != nil {
t.Fatal(err)
}
go func() {
// It's enough to have "talkative" process to stuck in the infinite loop of reading
for {
err := ep.Send("new line\n")
if err != nil {
return
}
}
}()

ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)
defer cancel()

_, err = ep.ExpectFunc(ctx, func(a string) bool { return false })

require.ErrorAs(t, err, &context.DeadlineExceeded)

if err = ep.Stop(); err != nil {
t.Fatal(err)
}
}

func TestEcho(t *testing.T) {
ep, err := NewExpect("echo", "hello world")
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/ctl_v3_elect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package e2e

import (
"context"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -106,7 +107,7 @@ func ctlV3Elect(cx ctlCtx, name, proposal string) (*expect.ExpectProcess, <-chan
return proc, outc, err
}
go func() {
s, xerr := proc.ExpectFunc(func(string) bool { return true })
s, xerr := proc.ExpectFunc(context.TODO(), func(string) bool { return true })
if xerr != nil {
cx.t.Errorf("expect failed (%v)", xerr)
}
Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/ctl_v3_lock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package e2e

import (
"context"
"fmt"
"os"
"strings"
Expand Down Expand Up @@ -127,7 +128,7 @@ func ctlV3Lock(cx ctlCtx, name string) (*expect.ExpectProcess, <-chan string, er
return proc, outc, err
}
go func() {
s, xerr := proc.ExpectFunc(func(string) bool { return true })
s, xerr := proc.ExpectFunc(context.TODO(), func(string) bool { return true })
if xerr != nil {
cx.t.Errorf("expect failed (%v)", xerr)
}
Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/v3_curl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package e2e

import (
"context"
"encoding/base64"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -249,7 +250,7 @@ func testV3CurlAuth(cx ctlCtx) {
testutil.AssertNil(cx.t, err)
defer proc.Close()

cURLRes, err := proc.ExpectFunc(lineFunc)
cURLRes, err := proc.ExpectFunc(context.Background(), lineFunc)
testutil.AssertNil(cx.t, err)

authRes := make(map[string]interface{})
Expand Down
3 changes: 2 additions & 1 deletion tests/framework/e2e/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package e2e

import (
"context"
"encoding/json"
"fmt"
"math/rand"
Expand All @@ -36,7 +37,7 @@ func WaitReadyExpectProc(exproc *expect.ExpectProcess, readyStrs []string) error
}
return false
}
_, err := exproc.ExpectFunc(matchSet)
_, err := exproc.ExpectFunc(context.Background(), matchSet)
return err
}

Expand Down

0 comments on commit 82bf79d

Please sign in to comment.