Update to cancel downstream operation in TimeoutStrategy.Pessimistic

[lor1mp]

This PR has changes to extend the life of the combinedTokenSource in the AsyncTimeoutEngine, so that the cancellation will get propegated downstream in the case of a TimeoutRejectedException.

Pull Request

The issue or feature being addressed

We noticed that the downstream cancellation token was never signaled in TimeoutStrategy.Pessimistic. The root cause seems to be the TimeoutRejectionException escaping the scope of the ImplementationAsync function on AsyncTimeoutEngine.cs:5. This causes the CancellationTokenSource to be disposed and the downstream token to never get canceled.

This is related to this issue: #722

Details on the issue fix or feature implementation

In this fix we explicitly cancel the combinedTokenSource as the function leaves scope, because without the exception will propagate and cause the source to be disposed before the downstream token can be canceled. This issue was demonstrated in a UT. However, that UT requires that we use "real time" as the TimeSpecBase.cs SystemClock overrides actually cancel the downstream token themselves on a timeout.

Confirm the following

• I started this PR by branching from the head of the default branch
• I have targeted the PR to merge into the default branch
• I have included unit tests for the issue/feature
• I have successfully run a local build

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (862690e) 84.63% compared to head (9a36f38) 84.65%.
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1697      +/-   ##
==========================================
+ Coverage   84.63%   84.65%   +0.02%     
==========================================
  Files         306      306              
  Lines        6819     6831      +12     
  Branches     1045     1047       +2     
==========================================
+ Hits         5771     5783      +12     
  Misses        839      839              
  Partials      209      209              

Flag	Coverage Δ
macos	84.65% <85.71%> (+0.02%)	⬆️
windows	84.65% <85.71%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/Polly/Timeout/AsyncTimeoutEngine.cs	93.10% <85.71%> (+0.51%)	⬆️

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lor1mp]

@martincostello - sorry this is my first PR into Polly. I'm not sure how to go about covering this line? (This is the line it's telling me I'm missing right?) Is it saying I don't have the false condition of the branch, and it's just way off?

[martincostello]

Don't worry about the coverage comment, it's more advisory as long as the PR status checks are green.

From looking at it, I think it's impossible to hit anyway.

[lor1mp]

Thanks for the approval @martincostello. Do I need to do anything about the code-ql failure? Is there anything left for me to do?

[martincostello]

No, it's just being especially flaky today and I'm retrying it until it passes 😅