Consider adding Socket Send/ReceiveAsync overloads that elide SocketFlags argument

[geoffkizer]

Background and Motivation

The sync methods for Send, Receive, SendTo, and ReceiveFrom all have overloads that don't take a SocketFlags argument. This argument is rarely used.

The Task-based async methods don't have these overloads. We actually approved overloads without SocketFlags for SendTo and ReceiveFrom in #938, but these haven't been implemented yet, and it doesn't really make sense to implement them just for SendTo and ReceiveFrom.

Proposed API

Additions to existing overloads

// Existing overloads in comments
public static class SocketTaskExtensions
{
//  public static Task<int> ReceiveAsync(this Socket socket, ArraySegment<byte> buffer, SocketFlags socketFlags);
    public static Task<int> ReceiveAsync(this Socket socket, ArraySegment<byte> buffer);
    
//  public static Task<int> ReceiveAsync(this Socket socket, IList<ArraySegment<byte>> buffers, SocketFlags socketFlags);
    public static Task<int> ReceiveAsync(this Socket socket, IList<ArraySegment<byte>> buffers);
    
//  public static ValueTask<int> ReceiveAsync(this Socket socket, Memory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken = default(CancellationToken));
    public static ValueTask<int> ReceiveAsync(this Socket socket, Memory<byte> buffer, CancellationToken cancellationToken = default(CancellationToken));
    
//  public static Task<SocketReceiveFromResult> ReceiveFromAsync(this Socket socket, ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEndPoint);
    public static Task<SocketReceiveFromResult> ReceiveFromAsync(this Socket socket, ArraySegment<byte> buffer, EndPoint remoteEndPoint);
    
//  public static Task<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(this Socket socket, ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEndPoint);
    public static Task<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(this Socket socket, ArraySegment<byte> buffer, EndPoint remoteEndPoint);
    
//  public static Task<int> SendAsync(this Socket socket, ArraySegment<byte> buffer, SocketFlags socketFlags);
    public static Task<int> SendAsync(this Socket socket, ArraySegment<byte> buffer);
    
//  public static Task<int> SendAsync(this Socket socket, IList<ArraySegment<byte>> buffers, SocketFlags socketFlags);
    public static Task<int> SendAsync(this Socket socket, IList<ArraySegment<byte>> buffers);
    
//  public static ValueTask<int> SendAsync(this Socket socket, ReadOnlyMemory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken = default(CancellationToken));
    public static ValueTask<int> SendAsync(this Socket socket, ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default(CancellationToken));
    
//  public static Task<int> SendToAsync(this Socket socket, ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP);
    public static Task<int> SendToAsync(this Socket socket, ArraySegment<byte> buffer, EndPoint remoteEP);
}

Additions to #938

// #938 overloads in comments
public static class SocketTaskExtensions
{
//  public static ValueTask<int> SendToAsync(this Socket socket, ReadOnlyMemory<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP, CancellationToken cancellationToken = default);
    public static ValueTask<int> SendToAsync(this Socket socket, ReadOnlyMemory<byte> buffer, EndPoint remoteEP, CancellationToken cancellationToken = default);

//  public static ValueTask<SocketReceiveFromResult> ReceiveFromAsync(this Socket socket, Memory<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP, CancellationToken cancellationToken = default);
    public static ValueTask<SocketReceiveFromResult> ReceiveFromAsync(this Socket socket, Memory<byte> buffer, EndPoint remoteEP, CancellationToken cancellationToken = default);

//  public static ValueTask<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(this Socket socket, Memory<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP, CancellationToken cancellationToken = default);
    public static ValueTask<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(this Socket socket, Memory<byte> buffer, EndPoint remoteEP, CancellationToken cancellationToken = default);
}

Consider: addition to #43933

This is not an async API but probably worth to add.

// #43933 overload in comments
public class Socket
{
//  public int ReceiveMessageFrom(Span<byte> buffer, ref SocketFlags socketFlags, ref EndPoint remoteEP, out IPPacketInformation ipPacketInformation);
    public int ReceiveMessageFrom(Span<byte> buffer, ref EndPoint remoteEP, out IPPacketInformation ipPacketInformation);
}

@scalablecory @wfurt @antonfirsov @stephentoub Thoughts?

[ghost]

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

[antonfirsov]

This argument is rarely used.

Agreed, this will simplify call sites and improve readability. I think we should go for an API proposal. Is it just an overlook that these overloads were missing from the original proposal for SendAsync/ReceiveAsync, or was there a practical reason?

[geoffkizer]

I don't think there was a practical reason, aside from just keeping the API surface minimal.

[scalablecory]

This seems reasonable to me, especially considering we have already approved UDP APIs with these overloads.

[karelz]

Triage: Let's add them from consistency reasons.
Should be simple once the API go through API approval.

[geoffkizer]

@antonfirsov Do you want to put together the actual API proposal?

[antonfirsov]

@geoffkizer edited the opening post, hope all overloads are included.

[geoffkizer]

The new overloads for SendToAsync, ReceiveFromAsync, and ReceiveMessageFromAsync in #33418 aren't included above. I think we should include them for completeness, even though they have technically been approved already.

[scalablecory]

Yeah. Also, we should make sure to merge this API back into #33418 once approved.

[antonfirsov]

Ok, will add tomorrow, thanks for checking!

[antonfirsov]

Done, PTAL

[geoffkizer]

Looks good

[antonfirsov]

According to #43901, we should move the proposed methods to the Socket class. Will update the proposal later.

[gfoidl]

Is it possible to use default arguments (socketFlags = SocketFlags.None) here?
This shouldn't be a breaking change, as the overload with socketFlags is already there.

For most overloads this could be used. Some of them have the socketFlags in a position where this wouldn't work, so the new extra overloads is needed. But where possible I prefer the default argument.

[geoffkizer]

My understanding is that isn't possible to do, though I'm not sure why. @terrajobst would know.

[geoffkizer]

I do wonder if we should wait a bit here until we figure out scatter/gather APIs. If we have new and improved scatter/gather APIs, we should only add overloads for those.

[bartonjs]

Video

• We discussed defaulting the parameter where possible, and landed on overloading for symmetry with existing things.
• We moved everything from the extensions class to the main class as instance methods
• Please double check that on ReceiveMessageFrom it's not important that the ref SocketFlags is not used, since that could have been passing data back to the caller.
• Also, let's paint the current extensions type as EB-Never

namespace System.Net.Sockets
{
    // Existing overloads in comments
    partial class Socket
    {
    //  public Task<int> ReceiveAsync(ArraySegment<byte> buffer, SocketFlags socketFlags);
        public Task<int> ReceiveAsync(ArraySegment<byte> buffer);
        
    //  public Task<int> ReceiveAsync(IList<ArraySegment<byte>> buffers, SocketFlags socketFlags);
        public Task<int> ReceiveAsync(IList<ArraySegment<byte>> buffers);
        
    //  public ValueTask<int> ReceiveAsync(Memory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken = default(CancellationToken));
        public ValueTask<int> ReceiveAsync(Memory<byte> buffer, CancellationToken cancellationToken = default(CancellationToken));
        
    //  public Task<SocketReceiveFromResult> ReceiveFromAsync(ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEndPoint);
        public Task<SocketReceiveFromResult> ReceiveFromAsync(ArraySegment<byte> buffer, EndPoint remoteEndPoint);
        
    //  public Task<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEndPoint);
        public Task<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(ArraySegment<byte> buffer, EndPoint remoteEndPoint);
        
    //  public Task<int> SendAsync(ArraySegment<byte> buffer, SocketFlags socketFlags);
        public Task<int> SendAsync(ArraySegment<byte> buffer);
        
    //  public Task<int> SendAsync(IList<ArraySegment<byte>> buffers, SocketFlags socketFlags);
        public Task<int> SendAsync(IList<ArraySegment<byte>> buffers);
        
    //  public ValueTask<int> SendAsync(ReadOnlyMemory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken = default(CancellationToken));
        public ValueTask<int> SendAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default(CancellationToken));
        
    //  public Task<int> SendToAsync(ArraySegment<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP);
        public Task<int> SendToAsync(ArraySegment<byte> buffer, EndPoint remoteEP);

    //  public ValueTask<int> SendToAsync(ReadOnlyMemory<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP, CancellationToken cancellationToken = default);
        public ValueTask<int> SendToAsync(ReadOnlyMemory<byte> buffer, EndPoint remoteEP, CancellationToken cancellationToken = default);

    //  public ValueTask<SocketReceiveFromResult> ReceiveFromAsync(Memory<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP, CancellationToken cancellationToken = default);
        public ValueTask<SocketReceiveFromResult> ReceiveFromAsync(Memory<byte> buffer, EndPoint remoteEP, CancellationToken cancellationToken = default);

    //  public ValueTask<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(Memory<byte> buffer, SocketFlags socketFlags, EndPoint remoteEP, CancellationToken cancellationToken = default);
        public ValueTask<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(Memory<byte> buffer, EndPoint remoteEP, CancellationToken cancellationToken = default);


/*** DOUBLE CHECK THAT THIS ONE IS OK, SINCE IT IS NO LONGER "RETURNING" DATA ***/

    //  public int ReceiveMessageFrom(Span<byte> buffer, ref SocketFlags socketFlags, ref EndPoint remoteEP, out IPPacketInformation ipPacketInformation);
        public int ReceiveMessageFrom(Span<byte> buffer, ref EndPoint remoteEP, out IPPacketInformation ipPacketInformation);
    }

    [EditorBrowsable(EditorBrowsableState.Never)]
    partial class SocketTaskExtensions
    {
    }
}

[hrrrrustic]

// public ValueTask<int> ReceiveAsync(Memory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken = default(CancellationToken));
(Third approved method)

There is no = default at existing method

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs

Lines 297 to 298 in 24971b0

	public ValueTask<int> ReceiveAsync(Memory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken) =>
	ReceiveAsync(buffer, socketFlags, fromNetworkStream: false, cancellationToken);

Should the new overload have = default? Is it safe to update the old one if needed?

[hrrrrustic]

Same for 8th method

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs

Line 442 in 24971b0

public ValueTask<int> SendAsync(ReadOnlyMemory<byte> buffer, SocketFlags socketFlags, CancellationToken cancellationToken)

[antonfirsov]

As long as it's there in the reference assembly declarations, it should work fine:

runtime/src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs

Line 386 in 24971b0

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; }

However we should have probably assigned the defaults in the implementation declarations too for clarity and consistency.