Implement DisconnectAsync method in SocketTaskExtensions

[gacardinal]

I was using the Socket extension methods provided by the SocketTaskExtensions class in System.Net.Sockets and noticed that almost all the *Async methods from Socket that are IO bound operations seemed to have their Task returning counterpart in SocketTaskExtensions except for DisconnectAsync.

Now, I've asked around online and people seemed to have mixed feelings about the issue. Some people didn't seem to understand why one would want to wait for the IO to complete when disconnecting a Socket. It's true that in most cases it's not really useful (and as I understand it, it's probably the reason it's not yet implemented in the framework) but even if use cases are limited, I would make the argument that for the sake of consistency, it'd be better that the SocketTaskExtensions class provides a DisconnectAsync method.

Implementation

My proposition is to add a DisconnectAsync method to the SocketTaskExtensions class.

    public static class SocketTaskExtensions
    {
        // ...
        public static Task DisconnectAsync(this Socket that, bool reuseSocket)
    }

The task is intentionally not returning a bool as its result (like the Socket.DisconnectAsync() is returning a bool to indicate whether the operation completed synchronously or not) because the goal of this extension method is to abstract that away.

Usage

A developer could use the API in the following way

    class Program
    {
        static async Task Main(string[] args)
        {
            Socket dummyReceiver = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            dummyReceiver.Bind(new IPEndPoint(IPAddress.Loopback, 7777));
            dummyReceiver.Listen(1);
            
            await Task.Run(() => {
                dummyReceiver.AcceptAsync();
                byte[] buffer = new byte[1000];
                dummyReceiver.ReceiveAsync(buffer, new SocketFlags());
            });

            Socket s = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

            s.Connect(new IPEndPoint(IPAddress.Loopback, 7777));

            await s.SendAsync(new byte[10000], new SocketFlags());
            await s.DisconnectAsync(false);
        }
    }

Additional context

Just to add some additional context, what led to me making this PR is this Stack Overflow question.

I would be happy to contribute an implementation for this feature!

[karelz]

Triage: We already have BeginDisconnect, so adding DisconnectAsync makes sense. Low pri.

[scalablecory]

Duplicate of #33417. Not really (using Github keyword) -- just consolidating socket API proposals to streamline for focused API review.

[geoffkizer]

I'm going to reopen this so we can do API review specifically on this (as opposed to all the APIs in #33417).

Also we should add a CancellationToken.

Updated proposal:

    public static class SocketTaskExtensions
    {
        // Existing APIs:
        // public IAsyncResult BeginDisconnect (bool reuseSocket, AsyncCallback callback, object state);
        // public void EndDisconnect (IAsyncResult asyncResult);

        public static Task DisconnectAsync(this Socket that, bool reuseSocket, CancellationToken cancellationToken);
    }

[terrajobst]

Video

• Looks good as proposed
• We should really consider "removing" SocketTaskExtensions
  • Merge with instance methods
  • Hide type & member
  • If we do this for .NET 6, we should delete DisconnectAsync from this type
  • @geoffkizer, could you file an issue for this?
• We should use ValueTask

namespace System.Net.Sockets
{
    public static class SocketTaskExtensions
    {
        // Existing APIs:
        // public IAsyncResult BeginDisconnect (bool reuseSocket, AsyncCallback callback, object state);
        // public void EndDisconnect (IAsyncResult asyncResult);

        public static ValueTask DisconnectAsync(this Socket socket, bool reuseSocket, CancellationToken cancellationToken=default);
    }
}

[geoffkizer]

We should really consider "removing" SocketTaskExtensions

Added #43901

[geoffkizer]

Note, per approval of #43901, this should be added as an instance method and not added to SocketTaskExtensions.

namespace System.Net.Sockets
{
    public partial class Socket
    {
        public ValueTask DisconnectAsync(bool reuseSocket, CancellationToken cancellationToken=default);
    }
}