move SocketTaskExtensions methods to Socket class #45083
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsFixes #43901 @antonfirsov @stephentoub @dotnet/ncl
|
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Net.Sockets/tests/FunctionalTests/ArgumentValidationTests.cs
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketTaskExtensions.cs
Show resolved
Hide resolved
| public System.Threading.Tasks.Task<System.Net.Sockets.Socket> AcceptAsync() { throw null; } | ||
| public System.Threading.Tasks.Task<System.Net.Sockets.Socket> AcceptAsync(System.Net.Sockets.Socket? acceptSocket) { throw null; } | ||
| public System.Threading.Tasks.Task ConnectAsync(System.Net.EndPoint remoteEP) { throw null; } | ||
| public System.Threading.Tasks.ValueTask ConnectAsync(System.Net.EndPoint remoteEP, System.Threading.CancellationToken cancellationToken) { throw null; } | ||
| public System.Threading.Tasks.Task ConnectAsync(System.Net.IPAddress address, int port) { throw null; } | ||
| public System.Threading.Tasks.ValueTask ConnectAsync(System.Net.IPAddress address, int port, System.Threading.CancellationToken cancellationToken) { throw null; } | ||
| public System.Threading.Tasks.Task ConnectAsync(System.Net.IPAddress[] addresses, int port) { throw null; } | ||
| public System.Threading.Tasks.ValueTask ConnectAsync(System.Net.IPAddress[] addresses, int port, System.Threading.CancellationToken cancellationToken) { throw null; } | ||
| public System.Threading.Tasks.Task ConnectAsync(string host, int port) { throw null; } | ||
| public System.Threading.Tasks.ValueTask ConnectAsync(string host, int port, System.Threading.CancellationToken cancellationToken) { throw null; } | ||
| public System.Threading.Tasks.Task<int> ReceiveAsync(System.ArraySegment<byte> buffer, System.Net.Sockets.SocketFlags socketFlags) { throw null; } | ||
| public System.Threading.Tasks.Task<int> ReceiveAsync(System.Collections.Generic.IList<System.ArraySegment<byte>> buffers, System.Net.Sockets.SocketFlags socketFlags) { throw null; } | ||
| public System.Threading.Tasks.ValueTask<int> ReceiveAsync(System.Memory<byte> buffer, System.Net.Sockets.SocketFlags socketFlags, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } | ||
| public System.Threading.Tasks.Task<System.Net.Sockets.SocketReceiveFromResult> ReceiveFromAsync(System.ArraySegment<byte> buffer, System.Net.Sockets.SocketFlags socketFlags, System.Net.EndPoint remoteEndPoint) { throw null; } | ||
| public System.Threading.Tasks.Task<System.Net.Sockets.SocketReceiveMessageFromResult> ReceiveMessageFromAsync(System.ArraySegment<byte> buffer, System.Net.Sockets.SocketFlags socketFlags, System.Net.EndPoint remoteEndPoint) { throw null; } | ||
| public System.Threading.Tasks.Task<int> SendAsync(System.ArraySegment<byte> buffer, System.Net.Sockets.SocketFlags socketFlags) { throw null; } | ||
| public System.Threading.Tasks.Task<int> SendAsync(System.Collections.Generic.IList<System.ArraySegment<byte>> buffers, System.Net.Sockets.SocketFlags socketFlags) { throw null; } | ||
| public System.Threading.Tasks.ValueTask<int> SendAsync(System.ReadOnlyMemory<byte> buffer, System.Net.Sockets.SocketFlags socketFlags, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } | ||
| public System.Threading.Tasks.Task<int> SendToAsync(System.ArraySegment<byte> buffer, System.Net.Sockets.SocketFlags socketFlags, System.Net.EndPoint remoteEP) { throw null; } |
There was a problem hiding this comment.
I suppose the additions were manual. Shouldn't we at least do to the alphabetical ordering to reduce the number of conflicts in later PR-s with consolidation?
|
Coming from #44889 (comment) ... shouldn't we also add xmldocs in this PR? |
Yup |
|
I suspect we already have docs for the extension methods, and we should just use them here. @carlossanlop Do you know? What should be done in this PR to preserve existing docs, just move them to these instance methods? |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@geoffkizer @antonfirsov If documentation was already added somewhere else for the APIs that were made public in this PR, you can copy the triple slash comments on top of these newly public APIs. |
|
@carlossanlop There are no triple-slash comments for the existing APIs. |
|
@geoffkizer Ah, got it. Then please add new triple slash comments. |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Doc comments added. @dotnet/ncl Can someone review? |
There was a problem hiding this comment.
If documentation text will be auto-generated, we should probably copy it from SocketTaskExtensions for consistency:
https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.sockettaskextensions.acceptasync?view=net-5.0#System_Net_Sockets_SocketTaskExtensions_AcceptAsync_System_Net_Sockets_Socket_
Otherwise LGTM.
| @@ -28,7 +28,18 @@ public partial class Socket | |||
| /// <summary>Cached instance for send operations that return <see cref="Task{Int32}"/>.</summary> | |||
| private TaskSocketAsyncEventArgs<int>? _multiBufferSendEventArgs; | |||
|
|
|||
| internal Task<Socket> AcceptAsync(Socket? acceptSocket) | |||
| /// <summary> | |||
| /// Accepts an incoming connection. | |||
There was a problem hiding this comment.
| /// Accepts an incoming connection. | |
| /// Performs an asynchronous operation to accept an incoming connection attempt on the socket. |
| /// <summary> | ||
| /// Accepts an incoming connection. | ||
| /// </summary> | ||
| /// <returns>An asynchronous task that completes with the accepted Socket.</returns> |
There was a problem hiding this comment.
| /// <returns>An asynchronous task that completes with the accepted Socket.</returns> | |
| /// <returns>An asynchronous task that completes with a <see cref="T:System.Net.Sockets.Socket" /> to handle communication with the remote host. |
| public Task<Socket> AcceptAsync() => AcceptAsync((Socket?)null); | ||
|
|
||
| /// <summary> | ||
| /// Accepts an incoming connection. |
There was a problem hiding this comment.
| /// Accepts an incoming connection. | |
| /// Performs an asynchronous operation on to accept an incoming connection attempt on the socket. |
| /// <summary> | ||
| /// Accepts an incoming connection. | ||
| /// </summary> | ||
| /// <param name="acceptSocket">The socket to use for accepting the connection.</param> |
There was a problem hiding this comment.
| /// <param name="acceptSocket">The socket to use for accepting the connection.</param> | |
| /// <param name="acceptSocket">The accepted Socket object. This value may be `null`.</param> |
| public Task ConnectAsync(IPAddress[] addresses, int port) => ConnectAsync(addresses, port, CancellationToken.None).AsTask(); | ||
|
|
||
| /// <summary> | ||
| /// Establishes a connection to a remote host. |
I used that as a starting point for what I added and tried to improve/clarify parts. |
ghost
locked as resolved and limited conversation to collaborators
Fixes #43901
@antonfirsov @stephentoub @dotnet/ncl