fix(#3975): do not unref timeout

[metcoder95]

This relates to...

Closes #3975

Rationale

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[ronag]

It should inred if the request is aborted

[metcoder95]

It should inred if the request is aborted

Do you mean to unref if the request can be aborted?
If the request was aborted, it will exit early and not reach the timeout 🤔

[ronag]

Consider the following case:

The retry timeout is active for another 60s. We wan't close the application by aborting all pending requests. With this change the application will continue hanging for 60s for no purpose.

[ronag]

Not perfect but probably better than current state of things...

[metcoder95]

Ah ok yes, see now.
Let me see if I can set a workaround here

[metcoder95]

It will require further thinking; I've tried to make it bare basic but it seems we are prone to several race conditions, so keeping it within the handler class might not sort out all the situations; let me spend more time on that later this week.

In the meanwhile, let's merge this one to unblock the issue and will continue further

[blinsay]

Appreciate the quick fix!

The retry timeout is active for another 60s. We wan't close the application by aborting all pending requests. With this change the application will continue hanging for 60s for no purpose.

Is destroy guaranteed to get called on the dispatcher in that scenario? Naively, it seems like stashing the timeout on the RetryHandler and deleting it on destroy seems appropriate. Not sure how hard that is though.