[Bugfix] Handle SocketException on accepted socket configuration#1648
[Bugfix] Handle SocketException on accepted socket configuration#1648hamdaankhalid wants to merge 1 commit intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the TCP accept loop in GarnetServerTcp by ensuring post-accept socket configuration and handler registration failures don’t terminate the accept loop, disposing failed sockets/handlers and continuing to accept new connections.
Changes:
- Wrap accepted-socket configuration (e.g.,
NoDelay,RemoteEndPointaccess) inside the existing handler creation/registration try/catch. - Ensure sockets are disposed when handler creation never occurs (e.g., dead socket timing window).
- Make handler cleanup more explicit (avoid null-conditional disposal paths).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
db8e542 to
65af274
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When a client RSTs between accept completing and socket setup (e.g. NoDelay), the SocketException was unhandled and crashed the process. Wrap the post-accept socket configuration in a try/catch, dispose the dead socket, and continue the accept loop. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
65af274 to
f27d3e2
Compare
| } | ||
| else | ||
| { | ||
| if (ex is SocketException se) |
There was a problem hiding this comment.
I know it would be cleaner to catch this exception, however I am not sure of what other exceptions are possible with the 2 lines I moved in the try catch block. So for now I am adding this if-else here as well.
Wrap the post-accept socket configuration in a try/catch, dispose the dead socket, and continue the accept loop. There is small gap where bad timing can still kill accept loop.