Ping.SendAsync() always successful, even if client is not pingable in cmd

I try to ping some IP addresses in C# using Ping.SendAsync() method. I have a treeView with 5 IP addresses and use the SendAsync() method for each node. Here you can see:

private void Form1_Load(object sender, EventArgs e)
{
    byte[] buffer = Encoding.ASCII.GetBytes(".");
    PingOptions options = new PingOptions(50, true);
    AutoResetEvent reset = new AutoResetEvent(false);
    Ping ping = new Ping();
    ping.PingCompleted += new PingCompletedEventHandler(ping_Complete);

    foreach (TreeNode node in treeView1.Nodes)
    {
        ping.SendAsync(node.Text, 5000, buffer, options, reset);
    }
}

private void ping_Complete(object sender, PingCompletedEventArgs k)
{
    foreach (TreeNode node in treeView1.Nodes)
    {
        PingReply reply = k.Reply;

        if (reply.Status == IPStatus.Success)
        {
            node.Text = node.Text + " (OK)";
        }

        else
        {
            node.Text = node.Text + " (FAILED)";
        }
    }
}

The problem is, the ping is always successful. I got 2 clients which were online and pingable. The other 3 are offline and not pingable (in cmd I couldn't ping these clients). So it should display this:

IP1 (OK)
IP2 (FAILED)
IP3 (FAILED)
IP4 (OK)
IP5 (FAILED)

But the output is 5 times "(OK)".

Any suggestions? :)

Jon Skeet
people
quotationmark

Every time you get any PingCompleted event, you're updating all the nodes in your tree the same way. Instead, you should only update the node corresponding to the IP address that the particular PingCompletedEventArgs corresponds with. You might want to use the node itself as the "state" argument in the SendAsync call, so that you can use it in your event handler.

My guess is that you're either getting failures and then updating everything with successes, or you're not waiting long enough to see the failures.

Just to validate that this is diagnostically correct, I suggest you separately log reply.Status somewhere that won't be overwritten.

Additionally, you're currently updating the UI from a non-UI thread, which is a very bad idea. You should marshal back to the UI thread before updating it.

people

See more on this question at Stackoverflow