Backport cancellation deadlock fix

[onyxmaster]

Hi. This is a backport of a (critical) cancellation deadlock fix (we were getting it multiple times a week with certain servers).

I had to backport some of the previous commits also to apply the deadlock fix since it was based on HTTP/2 class split HttpConnection->HttpConnectionBase.

[TalAloni]

Thanks for the PR, if there are mutliple fixes here I would prefer to have multiple PRs.
Also, IIUC, the issue require that you'll cancel some of the requests, is that right?

[onyxmaster]

Thanks for the PR, if there are mutliple fixes here I would prefer to have multiple PRs.

Quite fair, I'll keep this one focused on the cancellation deadlock fix.

Also, IIUC, the issue require that you'll cancel some of the requests, is that right?

Yes, bit since HttpClient builds it's own CancellationToken when Timeout property its set it fails not only for manually-initiated cancellations but for timeouts also, which are much more common. We started hitting the issue when one of the external services we rely upon got it's response time go up (they had some kind of a perf issue) and several instances of our backends died one by one over a course of a few hours, because of the deadlock since they were hitting timeouts almost for every request.

[TalAloni]

Sorry for taking the time on this.
Why is HttpConnectionBase needed to be backported?

[onyxmaster]

It's my turn to apologize for the long delay, I moved between 4 countries in last 2 weeks =)
Backporting this looked logical since most of the other changes assumed that HttpConnection was already separated into base and concrete class. Even if the HTTP/2 won't be backported, the change is trivial enough in my view to apply it.

[TalAloni]

Backporting this looked logical since most of the other changes assumed that HttpConnection was already separated into base and concrete class. Even if the HTTP/2 won't be backported, the change is trivial enough in my view to apply it.

HTTP/2 won't be backported to v2.2.0 (There is a 3.1 branch based on .NET Core 3.1 that does support HTTP/2)
Given this, seems that adding the abstraction is limiting more than adding. I would prefer to focus on the minimal subset of changes required to fix the bug and avoid anything that is outside that scope.

[onyxmaster]

I would prefer to focus on the minimal subset of changes required to fix the bug and avoid anything that is outside that scope.

While I understand that, I believe that if the backported code more closely matches the origin code (at least in terms of set of types) further backporting (even if it's bugfixes only) could be less complex.

Still it's your call, so I adjusted the PR to skip the introduction of the HttpConnectionBase class.

[TalAloni]

Merged. Thank you!

[TalAloni]

This change is included in v2.2.0.6