cenkalti / backoff

⏱ The exponential backoff algorithm in Go
https://pkg.go.dev/github.com/cenkalti/backoff/v4
MIT License
3.48k stars 186 forks source link

RetryNotify does not deal with operations that hang, even if a timeout is set #146

Open MattCosturos opened 1 week ago

MattCosturos commented 1 week ago

I have 0 experience developing in go, so please be gentle if I misunderstand any of the inner workings of multi-threaded go development. However to me, it appears that RetryNotify does not deal with operations that hang, even if a timeout is set.

RetryNotify uses RetryNotifyWithTimer uses doRetryNotify

doRetryNotify Line 87 starts a for loop. Line 88 calls the provided operation() This is a blocking call. It waits for operation to return, and checks the return value. Further down is a select block to wait for ctx.Done(), or the result of a timer. But if operation() on line 88 blocks we would never get here?

I would imagine you need some code like the following inside the for loop. Again, I have 0 experience in go, this is based off what I've learned in the past 24 hours.

    errCh := make(chan error)
    resCh := make(chan res)

    go func() {
        res, err := operation()
        if err != nil {
            errCh <- err
            return
        }
        resCh <- res
    }()

    select {
    case <-ctx.Done():
        return ctx.Err()
    case err := <- errCh
        //do the error logic here
    case res := <- resCh
        return res, nil
    }
MattCosturos commented 1 week ago

Submitted PR #147 which keeps functionality the same (all existing test cases pass) and adds a test for a blocked operation