fix(internal/async): fix deadlock by removing goroutine

The function may block longer than the timeout, because it waits until get() returns.

Also use synctest in order to make tests faster.

Author: pierrre

internal/async/wait.go

@@ -14,7 +14,6 @@ type IntervalStrategy func() <-chan time.Time
 
 // WaitSyncConfig defines the waiting options.
 type WaitSyncConfig struct {
-	// This method will be called from another goroutine.
 	Get              func() (value any, isTerminal bool, err error)
 	IntervalStrategy IntervalStrategy
 	Timeout          time.Duration
@@ -48,42 +47,22 @@ func WaitSync(config *WaitSyncConfig) (terminalValue any, err error) {
 		config.Timeout = defaultTimeout
 	}
 
-	resultValue := make(chan any)
-	resultErr := make(chan error)
-	timeout := make(chan bool)
+	timeoutCh := time.After(config.Timeout)
 
-	go func() {
-		for {
-			// get the payload
-			value, stopCondition, err := config.Get()
-			// send the payload
-			if err != nil {
-				resultErr <- err
-				return
-			}
-			if stopCondition {
-				resultValue <- value
-				return
-			}
-
-			// waiting for an interval before next get() call or a timeout
-			select {
-			case <-timeout:
-				return
-			case <-config.IntervalStrategy():
-				// sleep
-			}
+	for {
+		// get the payload
+		value, stopCondition, err := config.Get()
+		if err != nil {
+			return nil, err
+		}
+		if stopCondition {
+			return value, nil
 		}
-	}()
 
-	// waiting for a result or a timeout
-	select {
-	case val := <-resultValue:
-		return val, nil
-	case err := <-resultErr:
-		return nil, err
-	case <-time.After(config.Timeout):
-		timeout <- true
-		return nil, fmt.Errorf("timeout after %v", config.Timeout)
+		select {
+		case <-timeoutCh:
+			return nil, fmt.Errorf("timeout after %v", config.Timeout)
+		case <-config.IntervalStrategy(): // Sleep before next get() call.
+		}
 	}
 }

internal/async/wait_test.go

@@ -3,13 +3,12 @@ package async
 import (
 	"errors"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	"github.com/scaleway/scaleway-sdk-go/internal/testhelpers"
 )
 
-const flakiness = 500 * time.Millisecond
-
 type value struct {
 	doneIterations int
 	totalDuration  time.Duration
@@ -106,24 +105,36 @@ func TestWaitSync(t *testing.T) {
 			expValue: nil,
 			expErr:   errors.New("timeout after 2s"),
 		},
+		{
+			name: "WithError",
+			config: &WaitSyncConfig{
+				Get: func() (any, bool, error) {
+					return nil, false, errors.New("error")
+				},
+			},
+			expValue: nil,
+			expErr:   errors.New("error"),
+		},
 	}
 	for _, c := range testsCases {
 		c := c // do not remove me
 		t.Run(c.name, func(t *testing.T) {
 			t.Parallel()
 
-			terminalValue, err := WaitSync(c.config)
+			synctest.Test(t, func(t *testing.T) {
+				terminalValue, err := WaitSync(c.config)
 
-			testhelpers.Equals(t, c.expErr, err)
+				testhelpers.Equals(t, c.expErr, err)
 
-			if c.expValue != nil {
-				exp := c.expValue.(*value)
-				acc := terminalValue.(*value)
-				testhelpers.Equals(t, exp.doneIterations, acc.doneIterations)
+				if c.expValue != nil {
+					exp := c.expValue.(*value)
+					acc := terminalValue.(*value)
+					testhelpers.Equals(t, exp.doneIterations, acc.doneIterations)
 
-				ok := exp.totalDuration > acc.totalDuration-flakiness && exp.totalDuration < acc.totalDuration+flakiness
-				testhelpers.Assert(t, ok, "totalDuration don't match the target: (acc: %v, exp: %v)", acc.totalDuration, exp.totalDuration)
-			}
+					ok := exp.totalDuration == acc.totalDuration
+					testhelpers.Assert(t, ok, "totalDuration doesn't match the target: (acc: %v, exp: %v)", acc.totalDuration, exp.totalDuration)
+				}
+			})
 		})
 	}
 }