-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add new Task-based socket methods for UDP and reimplement existing ones using SocketAsyncEventArgs #47229
Conversation
# Conflicts: # src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs # src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketTaskExtensions.cs
@geoffkizer can you please take another look at the tests, and give a final approval if things look OK? I had to delete my SendTo cancellation test, since it looks like there is no reliable way to enforce I opened a bunch of issues for the corner cases discovered, and referenced them in my test workarounds. Most of them are minor compatibility cases, except the first one, which is actually concerning: #47342, #47335, #47469, #47425 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
src/libraries/System.Net.Sockets/tests/FunctionalTests/SendTo.cs
Outdated
Show resolved
Hide resolved
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.
One small nit above, otherwise LGTM.
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Closes #41502, but does not change the existing APM methods, I plan to do it in a follow-up PR.
The PR is built on @geoffkizer's branch, almost all product code changes were written by him, I just added a bunch of tests, fixed an issue with Windows cancellation (4e4add1), and moved extension methods to the
Socket
class as per #43901.As such, the product code needs a third pair of yes to review. @stephentoub maybe?