Move SocketTaskExtensions methods to Socket class itself

[geoffkizer]

Background and Motivation

For historical reasons, all of the Task-based APIs on Socket are defined as extension methods in SocketTaskExtensions.

We should move these methods to Socket itself, and mark the existing extension methods in SocketTaskExtensions as [EditorBrowsable(EditorBrowsableState.Never)].

Proposed API

namespace System.Net.Sockets
{
    public partial class Socket
    {
        // Existing SocketTaskExtensions APIs, converted to instance methods

        public Task<Socket> AcceptAsync();
        public Task<Socket> AcceptAsync(Socket? acceptSocket);

        public Task ConnectAsync(EndPoint remoteEP);
        public ValueTask ConnectAsync(EndPoint remoteEP, CancellationToken cancellationToken);
        public Task ConnectAsync(IPAddress address, int port);
        public ValueTask ConnectAsync(IPAddress address, int port, CancellationToken cancellationToken);
        public Task ConnectAsync(IPAddress[] addresses, int port);
        public ValueTask ConnectAsync(IPAddress[] addresses, int port, CancellationToken cancellationToken);
        public Task ConnectAsync(string host, int port);
        public ValueTask ConnectAsync(string host, int port, CancellationToken cancellationToken);

        public Task<int> ReceiveAsync(ArraySegment<byte> buffer, SocketFlags socketFlags);
        public ValueTask<int> ReceiveAsync(Memory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken = default);
        public Task<int> ReceiveAsync(IList<ArraySegment<byte>> buffers, SocketFlags socketFlags);
        public Task<SocketReceiveFromResult> ReceiveFromAsync(ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEndPoint);
        public Task<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEndPoint);

        public Task<int> SendAsync(ArraySegment<byte> buffer, SocketFlags socketFlags);
        public ValueTask<int> SendAsync(ReadOnlyMemory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken = default);
        public Task<int> SendAsync(IList<ArraySegment<byte>> buffers, SocketFlags socketFlags);
        public Task<int> SendToAsync(ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP);
    }
}

Note this change should apply to the following planned 6.0 APIs as well, which have already been approved as new extension methods on SocketTaskExtensions.

#1608
#42591
#33418

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[gfoidl]

A question on file-organisation. Add these to Socket.cs or a new partial class file (as Socket.cs is already huge)? Or to Socket.Tasks.cs?

[geoffkizer]

In most cases this is probably just making the existing internal APIs in Socket.Tasks.cs public, as the stuff in SocketTaskExtensions.cs is already just calling directly into those methods.

[bartonjs]

Video

Looks good as proposed

• Copy all members on SocketTaskExtensions to instance methods on Socket (removing the this parameter)
• Mark existing members as EditorBrowsable(Never)
• Remove any members on SocketTaskExtensions that have been introduced for .NET 6. Be nimble with regard to approved, but not-implemented ones.

[davidfowl]

Any reason to move the ArraySegment ones?

[geoffkizer]

Any reason to move the ArraySegment ones?

Mostly just consistency. I don't think there's any harm in it, is there?

[stephentoub]

Mostly just consistency. I don't think there's any harm in it, is there?

On the contrary, I expect we'll find it's otherwise a breaking change. Imagine code like:

ArraySegment<byte> segment = ...;
Task<int> t = socket.ReceiveAsync(segment, SocketFlags.None);

With both the Task<int> ReceiveAsync(ArraySegment<byte>, ...) and ValueTask<int> ReceiveAsync(Memory<byte>, ...) overloads as extensions, the above code will bind to the ReceiveAsync that accepts an ArraySegment<byte>. However, if just the Memory<byte>-based overload "moves" to be an instance method, with ArraySegment<byte> implicitly convertible to Memory<byte>, the compiler will then bind to the instance method, at which point the ValueTask<int> it returns will fail to cast to the Task<int> to which it's being assigned, and compilation will fail. If, however, both the ArraySegment<byte> and Memory<byte> overloads are added as instance methods, then the compiler will prefer the ArraySegment<byte> instance method, and things will continue to compile.

Someone can check me on that, but I'm pretty sure that's how things would play out. Yay complexities of overload resolution.