-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[browser] LoopbackServer - make GenericLoopbackServer.CloseWebSocket async #123327
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
[browser] LoopbackServer - make GenericLoopbackServer.CloseWebSocket async #123327
Conversation
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request converts synchronous socket operations to asynchronous ones to avoid calling blocking Task.WaitAll on browser platforms where it throws PlatformNotSupportedException. The changes rename CloseWebSocket() to CloseWebSocketAsync() and make the wrapper methods CloseAsync() and ShutdownAsync() async, then update all call sites throughout the test codebase.
Changes:
- Made
SocketWrapper.CloseAsync(),SocketWrapper.ShutdownAsync(), andSocketWrapper.CloseWebSocketAsync()async to avoid blocking calls - Converted
Http2LoopbackConnection.ShutdownSend()toShutdownSendAsync() - Updated all call sites across test files to await the newly async methods
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| GenericLoopbackServer.cs | Converted CloseWebSocket() to async CloseWebSocketAsync(), made Close() and Shutdown() async, removed blocking Task.WaitAll call |
| Http2LoopbackConnection.cs | Converted ShutdownSend() to async ShutdownSendAsync() with proper null checking |
| LoopbackServer.cs | Updated calls to closableWrapper.CloseAsync() and _socket.ShutdownAsync() to be awaited |
| SendReceiveTest.Loopback.cs | Updated socket shutdown/close calls to async versions |
| WebSocketHandshakeHelper.cs | Updated socket shutdown call to async version |
| SocketsHttpHandlerTest.cs | Updated socket/listen socket close and shutdown calls to async versions |
| MetricsTest.cs | Updated socket shutdown call to async version |
| HttpClientMiniStressTest.cs | Updated socket shutdown call to async version |
| HttpClientHandlerTest.ResponseDrain.cs | Updated listen socket close call to async version |
| HttpClientHandlerTest.RequestRetry.cs | Updated listen socket close call to async version |
| HttpClientHandlerTest.Http2.cs | Updated connection shutdown calls to async versions |
| HttpClientHandlerTest.AutoRedirect.cs | Updated socket shutdown calls to async versions |
src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…er.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
CarnaViire
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
looks ok :) |
Task.WaitAllon browser - it throws PNSE