Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Await for responseHeadersTask to observe exception if any#38358

Closed
wfurt wants to merge 1 commit intodotnet:masterfrom
wfurt:nre_38293
Closed

Await for responseHeadersTask to observe exception if any#38358
wfurt wants to merge 1 commit intodotnet:masterfrom
wfurt:nre_38293

Conversation

@wfurt
Copy link
Copy Markdown
Member

@wfurt wfurt commented Jun 7, 2019

There is race condition when we get RST from server. In this case the RST git during processing body but because we did not finish handing out HttpResponseMessage we faulted the receiving task. Since we fail to throw, we were hitting NRE in some unrelated code while trying to dereference response we never processed.

Since this depends on race in task processing I did not add new test.

fixes #38293

@wfurt wfurt requested review from a team and stephentoub June 7, 2019 23:14
@wfurt wfurt self-assigned this Jun 7, 2019
@davidsh davidsh changed the title await for responseHeadersTask to obesrve exception if any Await for responseHeadersTask to observe exception if any Jun 8, 2019
@davidsh davidsh added this to the 3.0 milestone Jun 8, 2019
}
else
{
await responseHeadersTask;
Copy link
Copy Markdown
Member

@JamesNK JamesNK Jun 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment explaining why (like what you said in the issue)

😄

else
{
await responseHeadersTask;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the await be moved down to below the ContinueWith and nulling out below?

@wfurt
Copy link
Copy Markdown
Member Author

wfurt commented Jun 14, 2019

I'll make this part of #38226

@wfurt wfurt closed this Jun 14, 2019
@wfurt wfurt deleted the nre_38293 branch June 14, 2019 19:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTTP/2 cancellation and object null reference

4 participants