-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
[Named Pipe] Check for pipe broken on WaitConnectionAsync #46182
Closed
simonferquel
wants to merge
25
commits into
dotnet:main
from
simonferquel:namedpipe/catch-io-exception-on-accept-async
Closed
[Named Pipe] Check for pipe broken on WaitConnectionAsync #46182
simonferquel
wants to merge
25
commits into
dotnet:main
from
simonferquel:namedpipe/catch-io-exception-on-accept-async
Conversation
This file contains 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
According to https://learn.microsoft.com/en-us/dotnet/api/system.io.pipes.namedpipeserverstream.waitforconnection?view=net-7.0#system-io-pipes-namedpipeserverstream-waitforconnection, WaitConnectionAsync can throw IOException when a pipe is broken. I am unable to write a test reproducing it, but I have seen it happen in the wild under high load with Unity's custom named pipe transport. Without this catch block, the server can unexpectedly stop accepting connections under high load. Signed-off-by: Simon Ferquel <simon.ferquel@unity3d.com>
simonferquel
requested review from
Tratcher,
halter73,
BrennanConroy,
JamesNK and
mgravell
as code owners
January 20, 2023 09:00
ghost
added
area-runtime
community-contribution
Indicates that the PR has been added by a community member
labels
Jan 20, 2023
Thanks for your PR, @simonferquel. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
* Run test on Linux and Mac * Use `ReplaceLineEndings` * Always use lf * Different approach * Revert * Doc
- rules would have assigned PRs to released milestones - note however that these rules don't presently seem to be working!! nits: - remove duplicate rule for release/6.0 PRs - remove rule for release/3.1 PRs since the release is no EOL
…tch (dotnet#45886) * Adding EnsureJsonTrimmability switch * Set TypeResolver to null * Removing RUC/RDC attributes * Removing ProblemDetails.Extension RUC/RDC * Adding Test remote execution support * Adding jsonoptions tests * Update ProblemDetails.cs * Update HttpValidationProblemDetailsJsonConverter.cs
It looks like the `addMilestone` task doesn't run when there is an existing milestone set. Clearing milestone before setting it to avoid this issue
* feat : Add `StringSyntax` for regex parameters * add missing using statement. * addressed PR feedback * Update RuleBuilder.cs * removed usage from test utils Co-authored-by: Javier Calvarro Nelson <jacalvar@microsoft.com>
* Add DispatchExceptionAsync * add testing * Update src/Components/Components/src/ComponentBase.cs Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com> * Remove ref since ComponentBase doesn't have access * Simplify ErrorBoundary test cases * API feedback: make RenderHandle.DispatchExceptionAsync internal * Revert "API feedback: make RenderHandle.DispatchExceptionAsync internal" This reverts commit c15f5fe. * Add unit test Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
…dotnet#46040) * Allow minimal host to be created without default HostBuilder behavior This adds a new Hosting API to reduce startup and app size, and ensures the default behavior is NativeAOT compatible. Fix dotnet#32485 * Use the new slim hosting API in the api template. Refactor the WebHostBuilder classes to share more code.
…117.5 (dotnet#46215) [main] Update dependencies from dotnet/arcade
The exact same thing exists 2 lines above that
…ached to the blazor app (dotnet#46132) * Press alt-shift-d and open firefox debug tab attached to the blazor app * remove debugger.launch. * removing unrelated changes * Removing unnecessary changes on chrome debugging. * addressing @mkArtakMSFT comments * Addressing @mkArtakMSFT comments. * Addressing Steve comments, adding a console.warning message and remove the beautiful message, removed the Newtonsoft from the send message, todo: remove the Newtonsoft from receive message. * Completely removing newtonsoft usage as asked by steve. * Change warning message.
Bumps [src/submodules/googletest](https://github.com/google/googletest) from `356fc30` to `ec25eea`. - [Release notes](https://github.com/google/googletest/releases) - [Commits](google/googletest@356fc30...ec25eea) --- updated-dependencies: - dependency-name: src/submodules/googletest dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…lder for java client (dotnet#46172) * Added withServerTimeout and withKeepAliveInterval to HubConnectionBuilder for java client
dotnet#46180) * Implement RouteHandlerServices.Map and use in RequestDelegateGenerator * Tweak generated code * Remove warnings from generated code * Fix docstring for RouteHandlerServices.Map * Add baseline tests and fix global:: issue * Clean up generated code * Add HTTP verb caching and fix usings * Use FQN and remove GeneratedCode attribute
Enumerate endpoint parameter names on startup instead of allocating a list.
@JamesNK PTAL |
…//github.com/simonferquel/aspnetcore into namedpipe/catch-io-exception-on-accept-async
JamesNK
requested review from
a team,
dougbu,
wtgodbe and
javiercn
as code owners
January 24, 2023 05:37
Sorry, I pushed to your PR branch to add logging and bought in a bunch of other commits by accident. I can't force push to your branch to clean up. I've created a new PR with your changes - #46230 |
2 tasks
amcasey
added
area-middleware
Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware
area-networking
Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
and removed
area-runtime
labels
Jun 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area-middleware
Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware
area-networking
Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
community-contribution
Indicates that the PR has been added by a community member
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.
#[Named Pipe] Check for pipe broken on WaitConnectionAsync
Description
According to https://learn.microsoft.com/en-us/dotnet/api/system.io.pipes.namedpipeserverstream.waitforconnection?view=net-7.0#system-io-pipes-namedpipeserverstream-waitforconnection, WaitConnectionAsync can throw IOException when a pipe is broken. I am unable to write a test reproducing it, but I have seen it happen in the wild under high load with Unity's custom named pipe transport.
Without this catch block, the server can unexpectedly stop accepting connections under high load.