Skip to content

[wasm] Cancel pending ReadableStream on Dispose or cancelationToken - #64285

Merged
pavelsavara merged 9 commits into
dotnet:mainfrom
pavelsavara:wasm_http_streaming_cancel
Jan 29, 2022
Merged

[wasm] Cancel pending ReadableStream on Dispose or cancelationToken#64285
pavelsavara merged 9 commits into
dotnet:mainfrom
pavelsavara:wasm_http_streaming_cancel

Conversation

@pavelsavara

@pavelsavarapavelsavara commented Jan 25, 2022•
edited
Loading

Copy link
Copy Markdown
Member
  • call ReadableStream.cancel on cancelationToken of WasmHttpReadStream.ReadAsync
  • call AbortController.abort when disposing WasmFetchResponse and WasmHttpReadStream
  • enable some of SocketsHttpHandler_HttpClientHandler_Http11_Cancellation_Test tests for Browser target
  • added WaitForClose to the LoopbackServer
  • added disconnect detection to RemoteLoopHandler
  • add unit test for the scenario

Fixes#64225

@pavelsavara
- call cancel when disposing ReadableStream
184846d
- add unit test for the scenario
@pavelsavarapavelsavara added this to the 7.0.0 milestone Jan 25, 2022
@pavelsavara
pavelsavara requested review from kg and marafJanuary 25, 2022 18:55
@ghost

ghost commented Jan 25, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details
  • call ReadableStream.cancel when disposing WasmHttpReadStream
  • call AbortController.abort when disposing WasmFetchResponse
  • add unit test for the scenario
Author:pavelsavara
Assignees:-
Labels:

arch-wasm, area-System.Net.Http, area-System.Runtime.InteropServices.JavaScript

Milestone:7.0.0

@pavelsavara

pavelsavara commented Jan 25, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-extra-platforms

@azure-pipelines

azure-pipelinesBot commented Jan 25, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

kg
kg reviewed Jan 25, 2022
View reviewed changes
Comment threadsrc/libraries/Common/tests/System/Net/Http/ResponseStreamTest.cs Outdated
@danmoseley

danmoseley commented Jan 25, 2022

Copy link
Copy Markdown
Contributor

One area- label only please, for labeler bot ..

halter73
halter73 reviewed Jan 25, 2022
View reviewed changes
Comment threadsrc/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs Outdated
@pavelsavara
- enabled cancelation tests for browser
19a8182
- fixed few cancelation issues
@pavelsavara

pavelsavara commented Jan 27, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-extra-platforms

@azure-pipelines

azure-pipelinesBot commented Jan 27, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavarapavelsavara changed the title [wasm] Cancel pending ReadableStream on Dispose[wasm] Cancel pending ReadableStream on Dispose or cancelationTokenJan 27, 2022
@pavelsavara

pavelsavara commented Jan 27, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-extra-platforms

@azure-pipelines

azure-pipelinesBot commented Jan 27, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

marek-safar
marek-safar reviewed Jan 27, 2022
View reviewed changes
Comment threadsrc/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs Outdated
@pavelsavara

pavelsavara commented Jan 28, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-extra-platforms

@azure-pipelines

azure-pipelinesBot commented Jan 28, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

maraf
maraf approved these changes Jan 28, 2022
View reviewed changes
Comment threadsrc/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs Outdated
lewing
lewing reviewed Jan 28, 2022
View reviewed changes
Comment threadsrc/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs Outdated
@lewing

lewing commented Jan 28, 2022

Copy link
Copy Markdown
Member

/azp run runtime-extra-platforms

@azure-pipelines

azure-pipelinesBot commented Jan 28, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
pavelsavara merged commit 7de2905 into dotnet:mainJan 29, 2022
@lewing

lewing commented Feb 17, 2022

Copy link
Copy Markdown
Member

We've had another customer report of this in net6 should we consider servicing it?

@ghostghost locked as resolved and limited conversation to collaborators Mar 19, 2022
@pavelsavara
pavelsavara deleted the wasm_http_streaming_cancel branch July 14, 2022 20:45
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lewinglewinglewing left review comments
@halter73halter73halter73 left review comments
@marek-safarmarek-safarmarek-safar left review comments
@marafmarafmaraf approved these changes
+1 more reviewer
@kgkgkg left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@pavelsavarapavelsavara

Labels

arch-wasmWebAssembly architecturearea-System.Net.Http

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

[Blazor WebAssembly] Support for Cancelling HTTP Requests by Diposing Response Stream

7 participants

@pavelsavara@danmoseley@lewing@halter73@kg@marek-safar@maraf