This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Release pinned buffers correctly for WinHttpHandler, WinHttpWebSocket #6500
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Both WinHttpHandler and WinHttpWebSocket use pinned buffers for receiving and sending data. The pinned GCHandle was being freed during the dispose of the WinHttpHandler or WinHttpWebSocket objects. The problem is that the buffer should not be unpinned until the async operation is done. It's possible for any async operation to be in flight (and where native code uses the buffer) when the object is explicitly or implicitly disposed. This fix moves the ownership of the pinned GCHandle to their respective state objects (WinHttRequestState, WinHttpWebSocketState). These objects are strongly rooted and are not released until all async operations are completed and the WinHTTP handles are fully closed. That makes these state objects a safe place to own and free the pinned buffers. Fixes #2508 and #6142
davidsh
added
area-System.Net
blocking
|
@stephentoub @CIPop PTAL |
| @@ -147,7 +163,17 @@ private void Dispose(bool disposing) | |||
|
|
|||
| if (_operationHandle.IsAllocated) | |||
| { | |||
| // This method only gets called when the WinHTTP request handle is fully closed and thus all | |||
There was a problem hiding this comment.
Is there anything we can assert to ensure that?
|
LGTM |
davidsh
added a commit
that referenced
this pull request
Feb 29, 2016
Release pinned buffers correctly for WinHttpHandler, WinHttpWebSocket
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
…ispose Release pinned buffers correctly for WinHttpHandler, WinHttpWebSocket Commit migrated from dotnet/corefx@378ce37
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Both WinHttpHandler and WinHttpWebSocket use pinned buffers for receiving and sending data. The pinned GCHandle was being freed during the dispose of the WinHttpHandler or WinHttpWebSocket objects. The problem is that the buffer should not be unpinned until the async operation is done. It's possible for any async operation to be in flight (and where native code uses the buffer) when the object is explicitly or implicitly disposed.
This fix moves the ownership of the pinned GCHandle to their respective state objects (WinHttRequestState, WinHttpWebSocketState). These objects are strongly rooted and are not released until all async operations are completed and the WinHTTP handles are fully closed. That makes these state objects a safe place to own and free the pinned buffers.
Fixes #2508 and #6142