fix: waiter#RunAndWait maybe cause deadlock. (#555)

for errChan cap is 1 and receive both event timeout err and callback err( block on the err for errChan handle receiving(wait#waitFunc) after callback send err(waiter#RunAndWait) ).

Author: liruohrh

waiter.go

@@ -175,7 +175,9 @@ func (w *waiter) reject(err error) {
 
 func newWaiter() *waiter {
 	w := &waiter{
-		errChan: make(chan error, 1),
+		// receive both event timeout err and callback err
+		// but just return event timeout err
+		errChan: make(chan error, 2),
 	}
 	return w
 }

waiter_test.go

@@ -1,7 +1,9 @@
 package playwright
 
 import (
+	"errors"
 	"fmt"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -136,3 +138,88 @@ func TestWaiterReturnErrorWhenMisuse(t *testing.T) {
 	_, err = waiter.Wait()
 	require.ErrorContains(t, err, "call RejectOnEvent before WaitForEvent")
 }
+
+func TestWaiterDeadlockForErrChanCapIs1AndCallbackErr(t *testing.T) {
+	// deadlock happen on waiter timeout before callback return err
+	waiterTimeout := 200.0
+	callbackTimeout := time.Duration(waiterTimeout+200.0) * time.Millisecond
+
+	mockCallbackErr := errors.New("mock callback error")
+
+	emitter := &eventEmitter{}
+	w := &waiter{
+		// just receive event timeout err or callback err
+		errChan: make(chan error, 1),
+	}
+
+	callbackOverCh := make(chan struct{})
+	callbackErrCh := make(chan error)
+	isAfterWaiterRunAndWaitExecuted := atomic.Bool{}
+	go func() {
+		_, err := w.WithTimeout(waiterTimeout).WaitForEvent(emitter, "", nil).RunAndWait(func() error {
+			time.Sleep(callbackTimeout)
+			close(callbackOverCh)
+			// block for this err, for waiter.errChan has cache event timeout err
+			return mockCallbackErr
+		})
+
+		isAfterWaiterRunAndWaitExecuted.Store(true)
+		callbackErrCh <- err
+	}()
+
+	// ensure waiter timeout
+	<-callbackOverCh
+	// give some time but never enough
+	time.Sleep(200 * time.Millisecond)
+
+	// Originally it was executed, but because waiter.errChan is currently caching the waiter timeout error,
+	// the callback error is blocked (because waitFunc has not been executed yet,
+	// waiter.errChan has not started receiving).
+	require.False(t, isAfterWaiterRunAndWaitExecuted.Load())
+
+	// if not receive waiter timeout error, isAfterWaiterRunAndWaitExecuted should be always false
+	err1 := <-w.errChan
+	require.ErrorIs(t, err1, ErrTimeout)
+
+	// for w.errChan cache is empty, callback error is sent and received, and then return it
+	err2 := <-callbackErrCh
+	require.ErrorIs(t, err2, mockCallbackErr)
+	require.True(t, isAfterWaiterRunAndWaitExecuted.Load())
+}
+
+func TestWaiterHasNotDeadlockForErrChanCapBiggerThan1AndCallbackErr(t *testing.T) {
+	// deadlock happen on waiter timeout before callback return err
+	waiterTimeout := 100.0
+	callbackTimeout := time.Duration(waiterTimeout+100.0) * time.Millisecond
+
+	mockCallbackErr := errors.New("mock callback error")
+
+	emitter := &eventEmitter{}
+	w := newWaiter()
+
+	callbackOverCh := make(chan struct{})
+	callbackErrCh := make(chan error)
+	isAfterWaiterRunAndWaitExecuted := atomic.Bool{}
+	go func() {
+		_, err := w.WithTimeout(waiterTimeout).WaitForEvent(emitter, "", nil).RunAndWait(func() error {
+			time.Sleep(callbackTimeout)
+			close(callbackOverCh)
+			return mockCallbackErr
+		})
+		isAfterWaiterRunAndWaitExecuted.Store(true)
+		callbackErrCh <- err
+	}()
+
+	// ensure waiter timeout
+	<-callbackOverCh
+
+	// for waiter.errChan cap is 2(greater than 1), so it will not block(deadlock)
+	require.Eventually(t,
+		func() bool { return isAfterWaiterRunAndWaitExecuted.Load() }, 100*time.Millisecond, 10*time.Microsecond)
+
+	// the first err still is waiter timeout, and is returned
+	err1 := <-w.errChan
+	require.ErrorIs(t, err1, mockCallbackErr)
+	err2 := <-callbackErrCh
+	require.ErrorIs(t, err2, ErrTimeout)
+}