Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add AcceptAsync cancellation overloads #53340

Merged
merged 4 commits into from
May 29, 2021

Conversation

geoffkizer
Copy link
Contributor

@geoffkizer geoffkizer commented May 27, 2021

Contributes to #33418
Fixes #40289

Add overloads to Socket.AcceptAsync that take a CancellationToken, and wire up support for cancellation.

AcceptAsync now uses the ValueTask-based mapping to SocketAsyncEventArgs (it previously used the Task-based mapping).

Optimize AcceptAsync for synchronous completion by deferring pinning of the accept buffer.

Minor SendFileAsync test cleanup from recent SendFileAsync PR.

Add similar overloads for TcpListener.

Fix the Unix implementation of named pipe servers to wire up cancellation via this new overload.

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented May 27, 2021

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

Issue Details

Contributes to #33418

Add overloads to Socket.AcceptAsync that take a CancellationToken, and wire up support for cancellation.

AcceptAsync now uses the ValueTask-based mapping to SocketAsyncEventArgs (it previously used the Task-based mapping).

Optimize AcceptAsync for synchronous completion by deferring pinning of the accept buffer.

Minor SendFileAsync test cleanup from recent SendFileAsync PR.

Author: geoffkizer
Assignees: -
Labels:

area-System.Net.Sockets, new-api-needs-documentation

Milestone: -

@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Nice. Generally looks good.

I was going to ask that you also fix #40289, but I see that CI flagged it anyway due to there now being a CancellationToken-based overload, and our analyzer that flags not correctly passing the token around kicking in.

@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@geoffkizer
Copy link
Contributor Author

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@geoffkizer
Copy link
Contributor Author

#40289 is now fixed, including enabling the relevant test on Unix.

I also added the corresponding TcpListener overloads.

PTAL.

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Assuming the test failures are all unrelated, LGTM.

@geoffkizer geoffkizer merged commit b5c91e4 into dotnet:main May 29, 2021
@geoffkizer geoffkizer deleted the acceptcancellation branch May 29, 2021 19:10
thaystg added a commit to thaystg/runtime that referenced this pull request Jun 1, 2021
…asm_debugger_and_use_debugger_agent

* upstream/main: (597 commits)
  Fix42292 (dotnet#52463)
  [mono] Remove some obsolete emscripten flags. (dotnet#53486)
  Fixed path to projects (dotnet#53435)
  support ServerCertificateContext in quic (dotnet#53175)
  Socket: delete unix local endpoint filename on Close (dotnet#52103)
  [mono] Fix sgen_gc_info.memory_load_bytes (dotnet#53364)
  Refactor MsQuic's native IP address types. (dotnet#53461)
  Re-enabled optimizations for gtFoldExprConst (dotnet#53347)
  Add diagnostic support into sample app and AppBuilders on Mono. (dotnet#53361)
  Fix issues with virtuals and mdarrays (dotnet#53400)
  Split Variant marshalling tests (dotnet#53035)
  Update clrjit.natvis to cover GT_SIMD and GT_HWINTRINSIC (dotnet#53470)
  remove WSL checks in tests (dotnet#53475)
  Always spawn message loop thread for SystemEvents (dotnet#53467)
  add AcceptAsync cancellation overloads (dotnet#53340)
  Remove unnecessary reference to iOS workload pack in the Mono workload (dotnet#53425)
  Add CookieContainer.GetAllCookies (dotnet#53441)
  Remove DynamicallyAccessedMembers on JsonSerializer  (dotnet#53235)
  [wasm] Re-enable Wasm.Build.Tests (dotnet#53433)
  [libraries] Move library tests Feature Switches defaults to Functional tests (dotnet#53253)
  ...
@ghost ghost locked as resolved and limited conversation to collaborators Jun 28, 2021
@karelz karelz added this to the 6.0.0 milestone Jul 15, 2021
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.

WaitForConnectionAsync does not respond to cancellation
3 participants