diff --git a/src/libraries/System.Net.Ping/ref/System.Net.Ping.cs b/src/libraries/System.Net.Ping/ref/System.Net.Ping.cs index 53053181659f3d..d549c5e306575a 100644 --- a/src/libraries/System.Net.Ping/ref/System.Net.Ping.cs +++ b/src/libraries/System.Net.Ping/ref/System.Net.Ping.cs @@ -62,10 +62,12 @@ public void SendAsyncCancel() { } public System.Threading.Tasks.Task SendPingAsync(System.Net.IPAddress address, int timeout) { throw null; } public System.Threading.Tasks.Task SendPingAsync(System.Net.IPAddress address, int timeout, byte[] buffer) { throw null; } public System.Threading.Tasks.Task SendPingAsync(System.Net.IPAddress address, int timeout, byte[] buffer, System.Net.NetworkInformation.PingOptions? options) { throw null; } + public System.Threading.Tasks.Task SendPingAsync(System.Net.IPAddress address, System.TimeSpan timeout, byte[]? buffer = null, System.Net.NetworkInformation.PingOptions? options = null, System.Threading.CancellationToken cancellationToken = default) { throw null; } public System.Threading.Tasks.Task SendPingAsync(string hostNameOrAddress) { throw null; } public System.Threading.Tasks.Task SendPingAsync(string hostNameOrAddress, int timeout) { throw null; } public System.Threading.Tasks.Task SendPingAsync(string hostNameOrAddress, int timeout, byte[] buffer) { throw null; } public System.Threading.Tasks.Task SendPingAsync(string hostNameOrAddress, int timeout, byte[] buffer, System.Net.NetworkInformation.PingOptions? options) { throw null; } + public System.Threading.Tasks.Task SendPingAsync(string hostNameOrAddress, System.TimeSpan timeout, byte[]? buffer = null, System.Net.NetworkInformation.PingOptions? options = null, System.Threading.CancellationToken cancellationToken = default) { throw null; } } public partial class PingCompletedEventArgs : System.ComponentModel.AsyncCompletedEventArgs { diff --git a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.OSX.cs b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.OSX.cs index 98d561df06e1b8..b1258f3e560161 100644 --- a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.OSX.cs +++ b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.OSX.cs @@ -22,17 +22,7 @@ public partial class Ping private static PingReply SendPingCore(IPAddress address, byte[] buffer, int timeout, PingOptions? options) => SendIcmpEchoRequestOverRawSocket(address, buffer, timeout, options); - private async Task SendPingAsyncCore(IPAddress address, byte[] buffer, int timeout, PingOptions? options) - { - Task t = SendIcmpEchoRequestOverRawSocketAsync(address, buffer, timeout, options); - PingReply reply = await t.ConfigureAwait(false); - - if (_canceled) - { - throw new OperationCanceledException(); - } - - return reply; - } + private Task SendPingAsyncCore(IPAddress address, byte[] buffer, int timeout, PingOptions? options) + => SendIcmpEchoRequestOverRawSocketAsync(address, buffer, timeout, options); } } diff --git a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.PingUtility.cs b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.PingUtility.cs index 7bdc4120cdb537..d887eb860be9d0 100644 --- a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.PingUtility.cs +++ b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.PingUtility.cs @@ -73,33 +73,35 @@ private PingReply SendWithPingUtility(IPAddress address, byte[] buffer, int time private async Task SendWithPingUtilityAsync(IPAddress address, byte[] buffer, int timeout, PingOptions? options) { - using (Process p = GetPingProcess(address, buffer, timeout, options)) + CancellationToken timeoutOrCancellationToken = _timeoutOrCancellationSource!.Token; + + using Process pingProcess = GetPingProcess(address, buffer, timeout, options); + pingProcess.Start(); + + try { - var processCompletion = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - p.EnableRaisingEvents = true; - p.Exited += (s, e) => processCompletion.SetResult(); - p.Start(); + await pingProcess.WaitForExitAsync(timeoutOrCancellationToken).ConfigureAwait(false); - try - { - await processCompletion.Task.WaitAsync(TimeSpan.FromMilliseconds(timeout)).ConfigureAwait(false); - } - catch (TimeoutException) - { - p.Kill(); - return CreatePingReply(IPStatus.TimedOut); - } + string stdout = await pingProcess.StandardOutput.ReadToEndAsync(timeoutOrCancellationToken).ConfigureAwait(false); - try + return ParsePingUtilityOutput(address, pingProcess.ExitCode, stdout); + } + catch (OperationCanceledException) when (timeoutOrCancellationToken.IsCancellationRequested) + { + if (!pingProcess.HasExited) { - string stdout = await p.StandardOutput.ReadToEndAsync().ConfigureAwait(false); - return ParsePingUtilityOutput(address, p.ExitCode, stdout); + pingProcess.Kill(); } - catch (Exception) + if (_canceled) { - // If the standard output cannot be successfully parsed, throw a generic PingException. - throw new PingException(SR.net_ping); + throw; } + return CreatePingReply(IPStatus.TimedOut); + } + catch (Exception e) + { + // If the standard output cannot be successfully read/parsed, throw a generic PingException. + throw new PingException(SR.net_ping, e); } } diff --git a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.RawSocket.cs b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.RawSocket.cs index 2e1b213bf7e950..471c7453b7d853 100644 --- a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.RawSocket.cs +++ b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.RawSocket.cs @@ -276,67 +276,63 @@ private static PingReply SendIcmpEchoRequestOverRawSocket(IPAddress address, byt } } - private static async Task SendIcmpEchoRequestOverRawSocketAsync(IPAddress address, byte[] buffer, int timeout, PingOptions? options) + private async Task SendIcmpEchoRequestOverRawSocketAsync(IPAddress address, byte[] buffer, int timeout, PingOptions? options) { SocketConfig socketConfig = GetSocketConfig(address, buffer, timeout, options); - using (Socket socket = GetRawSocket(socketConfig)) - { - int ipHeaderLength = socketConfig.IsIpv4 ? MinIpHeaderLengthInBytes : 0; - CancellationTokenSource timeoutTokenSource = new CancellationTokenSource(timeout); + using Socket socket = GetRawSocket(socketConfig); + int ipHeaderLength = socketConfig.IsIpv4 ? MinIpHeaderLengthInBytes : 0; - try + try + { + CancellationToken timeoutOrCancellationToken = _timeoutOrCancellationSource!.Token; + + await socket.SendToAsync( + socketConfig.SendBuffer.AsMemory(), + SocketFlags.None, + socketConfig.EndPoint, + timeoutOrCancellationToken) + .ConfigureAwait(false); + + byte[] receiveBuffer = new byte[2 * (MaxIpHeaderLengthInBytes + IcmpHeaderLengthInBytes) + buffer.Length]; + + // Read from the socket in a loop. We may receive messages that are not echo replies, or that are not in response + // to the echo request we just sent. We need to filter such messages out, and continue reading until our timeout. + // For example, when pinging the local host, we need to filter out our own echo requests that the socket reads. + long startingTimestamp = Stopwatch.GetTimestamp(); + while (!timeoutOrCancellationToken.IsCancellationRequested) { - await socket.SendToAsync( - new ArraySegment(socketConfig.SendBuffer), - SocketFlags.None, socketConfig.EndPoint, - timeoutTokenSource.Token) + SocketReceiveFromResult receiveResult = await socket.ReceiveFromAsync( + receiveBuffer.AsMemory(), + SocketFlags.None, + socketConfig.EndPoint, + timeoutOrCancellationToken) .ConfigureAwait(false); - byte[] receiveBuffer = new byte[2 * (MaxIpHeaderLengthInBytes + IcmpHeaderLengthInBytes) + buffer.Length]; - - // Read from the socket in a loop. We may receive messages that are not echo replies, or that are not in response - // to the echo request we just sent. We need to filter such messages out, and continue reading until our timeout. - // For example, when pinging the local host, we need to filter out our own echo requests that the socket reads. - long startingTimestamp = Stopwatch.GetTimestamp(); - while (!timeoutTokenSource.IsCancellationRequested) + int bytesReceived = receiveResult.ReceivedBytes; + if (bytesReceived - ipHeaderLength < IcmpHeaderLengthInBytes) { - SocketReceiveFromResult receiveResult = await socket.ReceiveFromAsync( - new ArraySegment(receiveBuffer), - SocketFlags.None, - socketConfig.EndPoint, - timeoutTokenSource.Token) - .ConfigureAwait(false); - - int bytesReceived = receiveResult.ReceivedBytes; - if (bytesReceived - ipHeaderLength < IcmpHeaderLengthInBytes) - { - continue; // Not enough bytes to reconstruct IP header + ICMP header. - } + continue; // Not enough bytes to reconstruct IP header + ICMP header. + } - if (TryGetPingReply(socketConfig, receiveBuffer, bytesReceived, startingTimestamp, ref ipHeaderLength, out PingReply? reply)) - { - return reply; - } + if (TryGetPingReply(socketConfig, receiveBuffer, bytesReceived, startingTimestamp, ref ipHeaderLength, out PingReply? reply)) + { + return reply; } } - catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut) - { - } - catch (SocketException ex) when (ex.SocketErrorCode == SocketError.MessageSize) - { - return CreatePingReply(IPStatus.PacketTooBig); - } - catch (OperationCanceledException) - { - } - finally - { - timeoutTokenSource.Dispose(); - } - - // We have exceeded our timeout duration, and no reply has been received. - return CreatePingReply(IPStatus.TimedOut); } + catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut) + { + } + catch (SocketException ex) when (ex.SocketErrorCode == SocketError.MessageSize) + { + return CreatePingReply(IPStatus.PacketTooBig); + } + catch (OperationCanceledException) when (!_canceled) + { + } + + // We have exceeded our timeout duration, and no reply has been received. + return CreatePingReply(IPStatus.TimedOut); } private static PingReply CreatePingReply(IPStatus status, IPAddress? address = null, long rtt = 0) diff --git a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Unix.cs b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Unix.cs index 987d0462d70cae..9a9bb7b167c649 100644 --- a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Unix.cs +++ b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Unix.cs @@ -27,20 +27,11 @@ private PingReply SendPingCore(IPAddress address, byte[] buffer, int timeout, Pi return reply; } - private async Task SendPingAsyncCore(IPAddress address, byte[] buffer, int timeout, PingOptions? options) + private Task SendPingAsyncCore(IPAddress address, byte[] buffer, int timeout, PingOptions? options) { - Task t = RawSocketPermissions.CanUseRawSockets(address.AddressFamily) ? + return RawSocketPermissions.CanUseRawSockets(address.AddressFamily) ? SendIcmpEchoRequestOverRawSocketAsync(address, buffer, timeout, options) : SendWithPingUtilityAsync(address, buffer, timeout, options); - - PingReply reply = await t.ConfigureAwait(false); - - if (_canceled) - { - throw new OperationCanceledException(); - } - - return reply; } } } diff --git a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.cs b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.cs index 2558f2d52801a1..adaa0495805ea8 100644 --- a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.cs +++ b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.cs @@ -3,6 +3,7 @@ using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Net.Sockets; using System.Threading; using System.Threading.Tasks; @@ -19,6 +20,8 @@ public partial class Ping : Component private SendOrPostCallback? _onPingCompletedDelegate; private bool _disposeRequested; private byte[]? _defaultSendBuffer; + private CancellationTokenSource? _timeoutOrCancellationSource; + // Used to differentiate between timeout and cancellation when _timeoutOrCancellationSource triggers private bool _canceled; // Thread safety: @@ -75,6 +78,7 @@ private void CheckDisposed() ObjectDisposedException.ThrowIf(_disposeRequested, this); } + [MemberNotNull(nameof(_timeoutOrCancellationSource))] private void CheckStart() { int currentStatus; @@ -83,6 +87,7 @@ private void CheckStart() currentStatus = _status; if (currentStatus == Free) { + _timeoutOrCancellationSource ??= new(); _canceled = false; _status = InProgress; _lockObject.Reset(); @@ -118,6 +123,10 @@ private void Finish() { Debug.Assert(_status == InProgress, $"Invalid status: {_status}"); _status = Free; + if (!_timeoutOrCancellationSource!.TryReset()) + { + _timeoutOrCancellationSource = null; + } _lockObject.Set(); } @@ -127,7 +136,6 @@ private void Finish() } } - // Cancels pending async requests, closes the handles. private void InternalDispose() { _disposeRequested = true; @@ -142,6 +150,8 @@ private void InternalDispose() _status = Disposed; } + _timeoutOrCancellationSource?.Dispose(); + InternalDisposeCore(); } @@ -320,33 +330,73 @@ public Task SendPingAsync(string hostNameOrAddress, int timeout, byte public Task SendPingAsync(IPAddress address, int timeout, byte[] buffer, PingOptions? options) { - CheckArgs(address, timeout, buffer, options); - return SendPingAsyncInternal(address, timeout, buffer, options); + return SendPingAsync(address, timeout, buffer, options, CancellationToken.None); } - private async Task SendPingAsyncInternal(IPAddress address, int timeout, byte[] buffer, PingOptions? options) + /// + /// Sends an Internet Control Message Protocol (ICMP) echo message with the specified data buffer to the computer that has the specified + /// , and receives a corresponding ICMP echo reply message from that computer as an asynchronous operation. This + /// overload allows you to specify a time-out value for the operation, a buffer to use for send and receive, control fragmentation and + /// Time-to-Live values, and a for the ICMP echo message packet. + /// + /// An IP address that identifies the computer that is the destination for the ICMP echo message. + /// The amount of time (after sending the echo message) to wait for the ICMP echo reply message. + /// + /// A array that contains data to be sent with the ICMP echo message and returned in the ICMP echo reply message. + /// The array cannot contain more than 65,500 bytes. + /// + /// A object used to control fragmentation and Time-to-Live values for the ICMP echo message packet. + /// The token to monitor for cancellation requests. The default value is . + /// The task object representing the asynchronous operation. + public Task SendPingAsync(IPAddress address, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null, CancellationToken cancellationToken = default) { - // Need to snapshot the address here, so we're sure that it's not changed between now - // and the operation, and to be sure that IPAddress.ToString() is called and not some override. - IPAddress addressSnapshot = GetAddressSnapshot(address); + return SendPingAsync(address, ToTimeoutMilliseconds(timeout), buffer ?? DefaultSendBuffer, options, cancellationToken); + } - CheckStart(); - try - { - Task pingReplyTask = SendPingAsyncCore(addressSnapshot, buffer, timeout, options); - return await pingReplyTask.ConfigureAwait(false); - } - catch (Exception e) when (e is not PlatformNotSupportedException) - { - throw new PingException(SR.net_ping, e); - } - finally - { - Finish(); - } + private Task SendPingAsync(IPAddress address, int timeout, byte[] buffer, PingOptions? options, CancellationToken cancellationToken) + { + CheckArgs(address, timeout, buffer, options); + + return SendPingAsyncInternal( + // Need to snapshot the address here, so we're sure that it's not changed between now + // and the operation, and to be sure that IPAddress.ToString() is called and not some override. + GetAddressSnapshot(address), + static (address, cancellationToken) => new ValueTask(address), + timeout, + buffer, + options, + cancellationToken); } public Task SendPingAsync(string hostNameOrAddress, int timeout, byte[] buffer, PingOptions? options) + { + return SendPingAsync(hostNameOrAddress, timeout, buffer, options, CancellationToken.None); + } + + /// + /// Sends an Internet Control Message Protocol (ICMP) echo message with the specified data buffer to the specified computer, and + /// receives a corresponding ICMP echo reply message from that computer as an asynchronous operation. This overload allows you to + /// specify a time-out value for the operation, a buffer to use for send and receive, control fragmentation and Time-to-Live values, + /// and a for the ICMP echo message packet. + /// + /// + /// The computer that is the destination for the ICMP echo message. The value specified for this parameter can be a host name or a + /// string representation of an IP address. + /// + /// The amount of time (after sending the echo message) to wait for the ICMP echo reply message. + /// + /// A array that contains data to be sent with the ICMP echo message and returned in the ICMP echo reply message. + /// The array cannot contain more than 65,500 bytes. + /// + /// A object used to control fragmentation and Time-to-Live values for the ICMP echo message packet. + /// The token to monitor for cancellation requests. The default value is . + /// The task object representing the asynchronous operation. + public Task SendPingAsync(string hostNameOrAddress, TimeSpan timeout, byte[]? buffer = null, PingOptions? options = null, CancellationToken cancellationToken = default) + { + return SendPingAsync(hostNameOrAddress, ToTimeoutMilliseconds(timeout), buffer ?? DefaultSendBuffer, options, cancellationToken); + } + + private Task SendPingAsync(string hostNameOrAddress, int timeout, byte[] buffer, PingOptions? options, CancellationToken cancellationToken) { if (string.IsNullOrEmpty(hostNameOrAddress)) { @@ -355,12 +405,19 @@ public Task SendPingAsync(string hostNameOrAddress, int timeout, byte if (IPAddress.TryParse(hostNameOrAddress, out IPAddress? address)) { - return SendPingAsync(address, timeout, buffer, options); + return SendPingAsync(address, timeout, buffer, options, cancellationToken); } CheckArgs(timeout, buffer, options); - return GetAddressAndSendAsync(hostNameOrAddress, timeout, buffer, options); + return SendPingAsyncInternal( + hostNameOrAddress, + static async (hostName, cancellationToken) => + (await Dns.GetHostAddressesAsync(hostName, cancellationToken).ConfigureAwait(false))[0], + timeout, + buffer, + options, + cancellationToken); } private static int ToTimeoutMilliseconds(TimeSpan timeout) @@ -379,9 +436,7 @@ public void SendAsyncCancel() { if (!_lockObject.IsSet) { - // As in the .NET Framework, this doesn't actually cancel an in-progress operation. It just marks it such that - // when the operation completes, it's flagged as canceled. - _canceled = true; + SetCanceled(); } } @@ -390,6 +445,12 @@ public void SendAsyncCancel() _lockObject.Wait(); } + private void SetCanceled() + { + _canceled = true; + _timeoutOrCancellationSource?.Cancel(); + } + private PingReply GetAddressAndSend(string hostNameOrAddress, int timeout, byte[] buffer, PingOptions? options) { CheckStart(); @@ -408,16 +469,30 @@ private PingReply GetAddressAndSend(string hostNameOrAddress, int timeout, byte[ } } - private async Task GetAddressAndSendAsync(string hostNameOrAddress, int timeout, byte[] buffer, PingOptions? options) + private async Task SendPingAsyncInternal( + TArg getAddressArg, + Func> getAddress, + int timeout, + byte[] buffer, + PingOptions? options, + CancellationToken cancellationToken) { + cancellationToken.ThrowIfCancellationRequested(); + CheckStart(); try { - IPAddress[] addresses = await Dns.GetHostAddressesAsync(hostNameOrAddress).ConfigureAwait(false); - Task pingReplyTask = SendPingAsyncCore(addresses[0], buffer, timeout, options); - return await pingReplyTask.ConfigureAwait(false); + using CancellationTokenRegistration _ = cancellationToken.UnsafeRegister(static state => ((Ping)state!).SetCanceled(), this); + + IPAddress address = await getAddress(getAddressArg, _timeoutOrCancellationSource.Token).ConfigureAwait(false); + + Task pingTask = SendPingAsyncCore(address, buffer, timeout, options); + // Note: we set the cancellation-based timeout only after resolving the address and initiating the ping with the + // intent that the timeout applies solely to the ping operation rather than to any setup steps. + _timeoutOrCancellationSource.CancelAfter(timeout); + return await pingTask.ConfigureAwait(false); } - catch (Exception e) when (e is not PlatformNotSupportedException) + catch (Exception e) when (e is not PlatformNotSupportedException && !(e is OperationCanceledException && _canceled)) { throw new PingException(SR.net_ping, e); } diff --git a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs index 253642e5f592f5..754a26a60fd4ed 100644 --- a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs +++ b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs @@ -12,6 +12,7 @@ using Xunit; using Xunit.Abstractions; +using System.Threading; namespace System.Net.NetworkInformation.Tests { @@ -663,6 +664,54 @@ await SendBatchPingAsync( }); } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + [InlineData(true)] + [InlineData(false)] + public async Task SendPingAsyncWithAlreadyCanceledToken(bool useIPAddress) + { + using CancellationTokenSource source = new(); + source.Cancel(); + + using Ping ping = new(); + Task pingTask = useIPAddress + ? ping.SendPingAsync(await TestSettings.GetLocalIPAddressAsync(), TimeSpan.FromSeconds(5), cancellationToken: source.Token) + : ping.SendPingAsync(TestSettings.LocalHost, TimeSpan.FromSeconds(5), cancellationToken: source.Token); + await Assert.ThrowsAnyAsync(() => pingTask); + Assert.True(pingTask.IsCanceled); + + // ensure that previous cancellation does not prevent future success + PingReply reply = useIPAddress + ? await ping.SendPingAsync(await TestSettings.GetLocalIPAddressAsync(), TimeSpan.FromSeconds(5)) + : await ping.SendPingAsync(TestSettings.LocalHost, TimeSpan.FromSeconds(5)); + Assert.Equal(IPStatus.Success, reply.Status); + } + + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + [OuterLoop] // Depends on external host and assumption that successful ping takes long enough for cancellation to go through first + public async Task CancelSendPingAsync(bool useIPAddress, bool useCancellationToken) + { + using CancellationTokenSource source = new(); + + using Ping ping = new(); + Task pingTask = useIPAddress + ? ping.SendPingAsync((await Dns.GetHostAddressesAsync(Test.Common.Configuration.Ping.PingHost))[0], TimeSpan.FromSeconds(5), cancellationToken: source.Token) + : ping.SendPingAsync(Test.Common.Configuration.Ping.PingHost, TimeSpan.FromSeconds(5), cancellationToken: source.Token); + if (useCancellationToken) + { + source.Cancel(); + } + else + { + ping.SendAsyncCancel(); + } + await Assert.ThrowsAnyAsync(() => pingTask); + Assert.True(pingTask.IsCanceled); + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))] [OuterLoop] // Depends on external host and assumption that network respects and does not change TTL public async Task SendPingToExternalHostWithLowTtlTest()