[API Proposal]: Add Ping.SendPingAsync overloads with CancellationToken

[deeprobin]

Background and motivation

This was already approved in #14336, but reverted in #67253 because this requires some more work.

See #64860 (comment)

API Proposal

namespace System.Net.NetworkInformation {
    public class Ping {
        public Task<PingReply> SendPingAsync(IPAddress address, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null, CancellationToken cancellationToken = default);
        public Task<PingReply> SendPingAsync(string hostNameOrAddress, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null, CancellationToken cancellationToken = default);
    }
}

/cc @danmoseley
/cc @stephentoub

[ghost]

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

Issue Details

---

Background and motivation

This was already approved in #14336, but reverted in #67253 because this requires some more work.

See #64860 (comment)

API Proposal

namespace System.Net.NetworkInformation {
    public class Ping {
       public Task<PingReply> SendPingAsync(IPAddress address, TimeSpan timeout, byte[]? buffer, PingOptions? options, CancellationToken cancellationToken);
       public Task<PingReply> SendPingAsync(string hostNameOrAddress, TimeSpan timeout, byte[]? buffer, PingOptions? options, CancellationToken cancellationToken);
    }
}

API Usage

Alternative Designs

No response

Risks

No response

Author:	deeprobin
Assignees:	-
Labels:	api-suggestion, area-System.Net, untriaged
Milestone:	-

[danmoseley]

Updated to match what was approved.

[deeprobin]

@danmoseley Thanks. I removed the trailing +. Can you remove the untriaged label?

[teo-tsirpanis]

How about returning ValueTask or accepting Memory<byte>? In the latter case PingReply.Buffer might need taking a look at. I haven't understood whether it contains the original buffer passed to the method. If it does, we can ignore it when constructing the resulting ping reply.

namespace System.Net.NetworkInformation;

public class Ping {
    public ValueTask<PingReply> SendPingAsync(IPAddress address, TimeSpan timeout, Memoty<byte> buffer = default, PingOptions? options = null, CancellationToken cancellationToken = default);
    public ValueTask<PingReply> SendPingAsync(string hostNameOrAddress, TimeSpan timeout, Memory<byte> buffer = default, PingOptions? options = null, CancellationToken cancellationToken = default);
}

[rhuijben]

Ping will almost always wait, so I think Task<> is fine. No need to optimize the no wait case

[sakno]

IMO, this proposal can be combined with #34856

[deeprobin]

@sakno Could be. But this is already approved and #34856 is not.

[madelson]

I have some interest in working on this.

One thought: could TimeSpan timeout be changed to TimeSpan? timeout = null? This would allow you to fall back to the default timeout as is possible with the int timeout APIs.

[deeprobin]

I have some interest in working on this.

@danmoseley can you assign this issue to him?

One thought: could TimeSpan timeout be changed to TimeSpan? timeout = null? This would allow you to fall back to the default timeout as is possible with the int timeout APIs.

@rhuijben Any thoughts? If we want to change the API shape, this API must be re-reviewed.