implement cancellation support for SendFileAsync and DisconnectAsync #53062
Conversation
… and rework some internal async logic to support this and reduce code duplication
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsAlso rework some internal async logic to support this and reduce code duplication.
|
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs
Outdated
Show resolved
Hide resolved
| /// <param name="overlapped">The overlapped that was used for this operation. Will be freed if the operation result will be handled synchronously.</param> | ||
| /// <param name="cancellationToken">The cancellation token to use to cancel the operation.</param> | ||
| /// <returns>The result status of the operation.</returns> | ||
| private unsafe SocketError ProcessIOCPResultWithCancellation(bool success, int bytesTransferred, NativeOverlapped* overlapped, CancellationToken cancellationToken) |
There was a problem hiding this comment.
It looks like this method is identical to the one above except that it doesn't do _singleBufferHandle = _buffer.Pin();? There's no way to consolidate them? Note that default(Memory<T>).Pin() should be very cheap.
There was a problem hiding this comment.
Yeah, you're right. I will consolidate them.
I could either pass default(Memory) or make the param nullable and pass null here.
There was a problem hiding this comment.
Not sure how would it look by passing Memory as an arg, but it can be also a bool arg:
private unsafe SocketError ProcessIOCPResultWithCancellation(
bool success,
int bytesTransferred,
NativeOverlapped* overlapped,
bool pinBuffer, // omit "_singleBufferHandle = _buffer.Pin();" if false
CancellationToken cancellationToken)There was a problem hiding this comment.
I plan to use this for cancellable Accept in the future, and I think that will work better if the buffer is passed in (it's not always just _buffer in the Accept case).
| { | ||
| // Note: We need to dispose of the overlapped iff the operation completed synchronously, | ||
| // and if we do, we must do so before we mark the operation as completed. | ||
| // Note: We need to dispose of the overlapped iff the operation result will be handled synchronously. |
There was a problem hiding this comment.
| // Note: We need to dispose of the overlapped iff the operation result will be handled synchronously. | |
| // Note: We need to dispose of the overlapped if the operation result will be handled synchronously. |
There was a problem hiding this comment.
This is actually intentional. "iff" is a bit obscure, mostly used in math contexts. If you think it's too obscure I can change it.
There was a problem hiding this comment.
Wasn't aware of this abbreviation, leave it as is, thanks for clarifying!
….Tasks.cs Co-authored-by: Stephen Toub <[email protected]>
…OCPResultWithCancellation into single method
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ghost
locked as resolved and limited conversation to collaborators
Also rework some internal async logic to support this and reduce code duplication.
Fixes #51452
Fixes #42591
@antonfirsov @stephentoub