Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passed cancellationToken might be default. #37441

Merged
merged 1 commit into from
Jun 5, 2020

Conversation

ManickaP
Copy link
Member

@ManickaP ManickaP commented Jun 4, 2020

In cases like WinHttpHandler (which is also build against .NET Framework), there is no way how to properly pass cancellationToken to content's CopyToAsync. The overloads accepting it are recent addition and not part of .NET Standard.
The tests were deadlocking waiting for cts.Task which is supposed to be finished from cancellationToken.Register(...) on the token that is not bound to test's TaskCompletionSource.

Fixes #36689

cc: @stephentoub

@ghost
Copy link

ghost commented Jun 4, 2020

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

@ManickaP ManickaP requested a review from a team June 4, 2020 19:34
@ManickaP
Copy link
Member Author

ManickaP commented Jun 4, 2020

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ManickaP
Copy link
Member Author

ManickaP commented Jun 4, 2020

BTW, does the test still make sense if we're bypassing the passed in cancellationToken?

@stephentoub
Copy link
Member

BTW, does the test still make sense if we're bypassing the passed in cancellationToken?

The test is named "PostAsync_Cancel_CancellationTokenPassedToContent"... if the cancellation token isn't passed to the content with WinHttpHandler, then we probably should just skip the test on WinHttpHandler ;)

@ManickaP
Copy link
Member Author

ManickaP commented Jun 5, 2020

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ManickaP
Copy link
Member Author

ManickaP commented Jun 5, 2020

Test skipped on WinHttpHandler completely.

ManickaP added a commit to ManickaP/runtime that referenced this pull request Jun 5, 2020
@ManickaP ManickaP merged commit f226796 into dotnet:master Jun 5, 2020
@ManickaP ManickaP deleted the mapichov/36689_default_ct branch June 5, 2020 11:44
@davidsh davidsh added this to the 5.0 milestone Jun 5, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WinHttpHandler PostAsync_Cancel_CancellationTokenPassedToContent test failing in CI
4 participants