Ping.SendAsync is getting stuck in infinite loop

Question

I'm trying to get the list of all hosts on my local network. Following this Stackoverflow thread. But Ping.SendAsync() is stuck in infinite loop even if I keep a very little timeout i.e., 20. Here is my code.

        static CountdownEvent countdown;
        static int upCount = 0;
        static object lockObj = new object();
        const bool resolveNames = false;
        static List<string> activeInterfaces = new List<string>();

        static void p_PingCompleted(object sender, PingCompletedEventArgs e)
        {
            try
            {

                string ip = (string)e.UserState;
                if (e.Reply != null && e.Reply.Status == IPStatus.Success)
                {
                    //if (resolveNames)
                    //{
                    //    string name;
                    //    try
                    //    {
                    //        IPHostEntry hostEntry = Dns.GetHostEntry(ip);
                    //        name = hostEntry.HostName;
                    //    }
                    //    catch (SocketException ex)
                    //    {
                    //        name = "?";
                    //    }
                    //    Console.WriteLine("{0} ({1}) is up: ({2} ms)", ip, name, e.Reply.RoundtripTime);
                    //}
                    //else
                    //{
                    activeInterfaces.Add(ip);
                    Console.WriteLine("{0} is up: ({1} ms)", ip, e.Reply.RoundtripTime);
                    //}
                    lock (lockObj)
                    {
                        upCount++;
                    }
                }
                else if (e.Reply == null)
                {
                    Console.WriteLine("Pinging {0} failed. (Null Reply object?)", ip);
                }
                countdown.Signal();
            }
            catch (Exception exp)
            {
                Console.WriteLine("Here you go...");
            }

        }


        [HttpGet]
        [Route("api/pc/getOnlinePCs")]
        public List<string> GetOnlinePCs()
        {

            activeInterfaces.Clear();
            //List<string> activeInterfaces=new List<string>();
            string ipBase = "";

            var host = Dns.GetHostEntry(Dns.GetHostName());
            foreach (var ip in host.AddressList)
            {
                if (ip.AddressFamily == AddressFamily.InterNetwork)
                {
                    ipBase = ip.ToString().Substring(0, (ip.ToString().LastIndexOf(".") + 1));//"10.22.4.";

                }
            }


            countdown = new CountdownEvent(1);
            Stopwatch sw = new Stopwatch();
            sw.Start();
            for (int i = 2; i < 254; i++)
            {
                string ip = ipBase + i.ToString();

                //var tcpClient = new TcpClient();
                //tcpClient.Connected += new PingCompletedEventHandler(p_PingCompleted);
                Ping ping = new Ping();
                ping.PingCompleted += new PingCompletedEventHandler(p_PingCompleted);
                countdown.AddCount();
                ping.SendAsync(ip, 20, ip);
            }
            countdown.Signal();
            countdown.Wait();
            sw.Stop();
            TimeSpan span = new TimeSpan(sw.ElapsedTicks);
            Console.WriteLine("Took {0} milliseconds. {1} hosts active.", sw.ElapsedMilliseconds, upCount);
            Console.ReadLine();
     

            return activeInterfaces;
        }

I don't think the solution provided in SO thread is expired, rather I would be making a small logical mistake as I'm a front-end guy. Thanks!

Edit

As per an expert comment, I wrapped the SendAsync code in try/catch and finally block as

              try
                {
                    Ping ping = new Ping();
                    ping.PingCompleted += new PingCompletedEventHandler(p_PingCompleted);
                    countdown.AddCount();
                    ping.SendAsync(ip, 20, ip);
                }
                catch(Exception exception)
                {
                    Console.WriteLine($"Exception on {ip} .Technical details: {exception}");
                }
                finally
                {
                    countdown.AddCount();
                }

But when I debug the code it always fall to finally block. Not to exception block and after the 253 pings iteration completed, again infinite loop! @Evk and @jdweng Please Help!

@Evk here is the image of how it's stuck

Answer 1

Ping SendAsync is not doing what you expect, you can release the resource however you need to deal with the answer coming back from another thread yourself, you need to hook PingCompleted to deal with the answer.

You are better off making an async method of your method by calling ping and in your method and call your method in the async method by implementing it yourself https://learn.microsoft.com/en-us/dotnet/standard/parallel-programming/how-to-wrap-eap-patterns-in-a-task

Ping pingSender = new Ping ();

// When the PingCompleted event is raised,
// the PingCompletedCallback method is called.
pingSender.PingCompleted += new PingCompletedEventHandler (PingCompletedCallback);

// Create a buffer of 32 bytes of data to be transmitted.
string data = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
byte [] buffer = Encoding.ASCII.GetBytes (data);

// Wait 12 seconds for a reply.
int timeout = 12000;

// Set options for transmission:
// The data can go through 64 gateways or routers
// before it is destroyed, and the data packet
// cannot be fragmented.
PingOptions options = new PingOptions (64, true);

// Send the ping asynchronously.
// Use the waiter as the user token.
// When the callback completes, it can wake up this thread.
pingSender.SendAsync (who, timeout, buffer, options, waiter);

// Prevent this example application from ending.
// A real application should do something useful
// when possible.
waiter.WaitOne ();

Answer 2

It's not answering your question but answers your tasks I think, You're brute-force looking at IP addresses using an IP request, better use the Address Resolution Protocol also known as ARP.

First, create a structure to store the data:

public struct MacIpPair : IEquatable<MacIpPair>
{
    /// <summary>
    /// Initializes a new instance of the <see cref="MacIpPair"/> struct.
    /// </summary>
    /// <param name="mac">The mac.</param>
    /// <param name="ip">The ip.</param>
    /// <param name="type">ARP record status</param>
    /// <exception cref="System.ArgumentException">Mac address needs to be provided - mac</exception>
    /// <exception cref="System.ArgumentException">IP address needs to be provided - ip</exception>
    public MacIpPair(string mac, string ip, ArpStatus type)
    {
        if (string.IsNullOrEmpty(mac))
        {
            throw new System.ArgumentException("Mac address needs to be provided", nameof(mac));
        }

        if (string.IsNullOrEmpty(ip))
        {
            throw new System.ArgumentException("IP address needs to be provided", nameof(ip));
        }

        MacAddress = mac;
        IpAddress = ip;
        Status = type;
    }

    /// <summary>
    /// The mac address
    /// </summary>
    /// <value>The mac address.</value>
    [NotNull]
    public string MacAddress { get; }

    /// <summary>
    /// The ip address
    /// </summary>
    /// <value>The ip address.</value>
    [NotNull]
    public string IpAddress { get; }
    /// <summary>
    /// The status of the ARP entry
    /// </summary>
    ArpStatus Status { get; }

    /// <summary>
    /// Determines whether the specified <see cref="System.Object" /> is equal to this instance.
    /// </summary>
    /// <param name="obj">The object to compare with the current instance.</param>
    /// <returns><c>true</c> if the specified <see cref="System.Object" /> is equal to this instance; otherwise, <c>false</c>.</returns>
    public override bool Equals(object? obj)
    {
        return obj is MacIpPair pair && Equals(pair);
    }

    /// <summary>
    /// Indicates whether the current object is equal to another object of the same type.
    /// </summary>
    /// <param name="other">An object to compare with this object.</param>
    /// <returns><see langword="true" /> if the current object is equal to the <paramref name="other" /> parameter; otherwise, <see langword="false" />.</returns>
    public bool Equals(MacIpPair other)
    {
        return MacAddress == other.MacAddress &&
                IpAddress == other.IpAddress &&
                Status==other.Status
                ;
    }

    /// <summary>
    /// Returns a hash code for this instance.
    /// </summary>
    /// <returns>A hash code for this instance, suitable for use in hashing algorithms and data structures like a hash table.</returns>
    public override int GetHashCode()
    {
        return HashCode.Combine(MacAddress, IpAddress, Status);
    }

    /// <summary>
    /// Implements the == operator.
    /// </summary>
    /// <param name="left">The left.</param>
    /// <param name="right">The right.</param>
    /// <returns>The result of the operator.</returns>
    public static bool operator ==(MacIpPair left, MacIpPair right)
    {
        return left.Equals(right);
    }

    /// <summary>
    /// Implements the != operator.
    /// </summary>
    /// <param name="left">The left.</param>
    /// <param name="right">The right.</param>
    /// <returns>The result of the operator.</returns>
    public static bool operator !=(MacIpPair left, MacIpPair right)
    {
        return !(left == right);
    }
}

Having the structure you can fill it:

/// <summary>
/// Gets all mac addresses and IP PAIRS visible to the computer.
/// </summary>
/// <remarks>
/// This will use RRP broadcast to obtain the mac addresses for all devices connected with the DHCP server
/// </remarks>
/// <returns>List&lt;MacIpPair&gt;.</returns>
public static List<MacIpPair> GetAllMacAddressesAndIpPairs()
{
    try
    {
        List<MacIpPair> mip = new List<MacIpPair>();
        using System.Diagnostics.Process pProcess = new System.Diagnostics.Process();
        pProcess.StartInfo.FileName = "arp";
        pProcess.StartInfo.Arguments = "-a ";
        pProcess.StartInfo.UseShellExecute = false;
        pProcess.StartInfo.RedirectStandardOutput = true;
        pProcess.StartInfo.CreateNoWindow = true;
        pProcess.Start();
        pProcess.WaitForExit();
        string cmdOutput = pProcess.StandardOutput.ReadToEnd();
        string pattern = @"(?<ip>([0-9]{1,3}\.?){4})\s*(?<mac>([a-f0-9]{2}-?){6})";

        foreach (Match? m in Regex.Matches(cmdOutput, pattern, RegexOptions.IgnoreCase))
        {
            if (m is null)
                continue;
            mip.Add(new MacIpPair(m.Groups["mac"].Value, m.Groups["ip"].Value, ArpStatus.Static));
        }

        return mip;
    }
    catch (Exception e)
    {
        Walter.TicketService.PostException(e);
        throw;
    }
}

This code is from my Nuget package Walter.Net.Networking using the class Walter.Net.Networking.LocalNetwork,

I have an additional wrapper calling native windows method in case the above wrapper fails.

[DllImport("IpHlpApi.dll", EntryPoint = "GetIpNetTable")]
[return: MarshalAs(UnmanagedType.U4)]
internal static extern int GetIpNetTable(IntPtr pIpNetTable, [MarshalAs(UnmanagedType.U4)] ref int pdwSize, bool bOrder);