Skip to content

Conversation

@wfurt
Copy link
Member

@wfurt wfurt commented Oct 25, 2023

it seems like the callback can be called multiple times - perhaps because of TLS 1.3.
With that, it is called and then it is set to null in CreateResponseMessage

state.RequestHandle = null; // ownership successfully transferred to WinHttpResponseStram.

It seems like this is only problem with Debug build. All the methods inside OnRequestSendingRequest can handle null RequestHandle without crashing. However the processing seems unnecessary so I simply removed the assert and return from the method under this condition.

This was somewhat easy to reproduce on my Windows VM. After the change all tests are passing e.g. the Assert really seems unnecessary.

fixes #93099

@wfurt wfurt added this to the 9.0.0 milestone Oct 25, 2023
@wfurt wfurt requested a review from a team October 25, 2023 17:44
@wfurt wfurt self-assigned this Oct 25, 2023
@ghost
Copy link

ghost commented Oct 25, 2023

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

Issue Details

it seems like the callback can be called multiple times - perhaps because of TLS 1.3.
With that, it is called and then it is set to null in CreateResponseMessage

state.RequestHandle = null; // ownership successfully transferred to WinHttpResponseStram.

It seems like this is only problem with Debug build. All the methods inside OnRequestSendingRequest can handle null RequestHandle without crashing. However the processing seems unnecessary so I simply removed the assert and return from the method under this condition.

This was somewhat easy to reproduce on my Windows VM. After the change all tests are passing e.g. the Assert really seems unnecessary.

fixes #93099

Author: wfurt
Assignees: wfurt
Labels:

area-System.Net.Http

Milestone: 9.0.0

@wfurt wfurt merged commit 3e708f2 into dotnet:main Oct 27, 2023
@wfurt wfurt deleted the winHttpAssert2 branch October 27, 2023 21:58
liveans pushed a commit to liveans/dotnet-runtime that referenced this pull request Nov 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Nov 27, 2023
@ManickaP
Copy link
Member

/backport to release/8.0-staging

@github-actions
Copy link
Contributor

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/13499278819

@github-actions github-actions bot unlocked this conversation Feb 24, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2025
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] Assertion in AfterReadResponseServerError_ClientWrite

2 participants