-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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] WS cancelation fix #99522
[browser] WS cancelation fix #99522
Conversation
@@ -135,9 +135,10 @@ private WebSocketState FastState | |||
|
|||
internal Task ConnectAsync(Uri uri, List<string>? requestedSubProtocols, CancellationToken cancellationToken) | |||
{ | |||
AbortIfCancelationRequested(cancellationToken); |
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.
Why moving it out of the lock?
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.
It probably doesn't matter
{ | ||
Abort(); | ||
} // lock | ||
cancellationToken.ThrowIfCancellationRequested(); |
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.
fyi: this internally checks for 'cancellationToken.IsCancellationRequested`.
runtime/src/libraries/System.Private.CoreLib/src/System/Threading/CancellationToken.cs
Line 359 in efa2b78
public void ThrowIfCancellationRequested() |
...System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/BrowserWebSocket.cs
Outdated
Show resolved
Hide resolved
…ockets/BrowserWebSockets/BrowserWebSocket.cs Co-authored-by: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com>
Fixes #98201
Fixes #98145