Ping causes high cpu when disconnected from network

Question

I'm making a C# Pinger in WPF using Ping() and BackgroundWorker(). Here is my full code:

using System;
using System.ComponentModel;
using System.Net.NetworkInformation;
using System.Runtime.InteropServices;
using System.Threading;
using System.Windows;
using System.Windows.Media;

namespace Pinger
{
    public partial class MainWindow : Window
    {
        [DllImport("wininet.dll")] private static extern bool InternetGetConnectedState(out int Description, int ReservedValue);

        public static bool IsConnectedToInternet() { return InternetGetConnectedState(out _, 0); }

        private readonly BackgroundWorker BackWork_Pinger = new BackgroundWorker();

        private int counter = 0; private readonly Ping pinger = new Ping();

        public MainWindow()
        {
            InitializeComponent();

            BackWork_Pinger.WorkerReportsProgress = true;

            BackWork_Pinger.DoWork += (_, args) => { for (int i = 1; i > 0; ++i) { if (IsConnectedToInternet()) { BackWork_Pinger.ReportProgress(i); Thread.Sleep(100); } } };

            BackWork_Pinger.ProgressChanged += (_, args) =>
            {
                try
                {
                    PingReply pingReply = pinger.Send("8.8.8.8");

                    if (pingReply != null)
                    {
                        switch (pingReply.Status)
                        {
                            case IPStatus.Success:

                                int PingNumber = Convert.ToInt32(pingReply.RoundtripTime);

                                if (PingNumber < 100) { Ping_Value.Foreground = Brushes.Lime; Ping_Value.Text = PingNumber + " ms"; }

                                else if (PingNumber < 300) { Ping_Value.Foreground = Brushes.Yellow; Ping_Value.Text = PingNumber + " ms"; }

                                else if (PingNumber < 500) { Ping_Value.Foreground = Brushes.DarkOrange; Ping_Value.Text = PingNumber + " ms"; }

                                else { Ping_Value.Foreground = Brushes.Red; Ping_Value.Text = "Bad"; }

                                break;

                            case IPStatus.TimedOut: Ping_Value.Foreground = Brushes.WhiteSmoke; Ping_Value.Text = "Disconnected"; break;

                            default: Ping_Value.Foreground = Brushes.WhiteSmoke; Ping_Value.Text = "Disconnected"; break;
                        }
                    }
                    else { Ping_Value.Foreground = Brushes.WhiteSmoke; Ping_Value.Text = "Disconnected"; }
                }
                catch { Ping_Value.Foreground = Brushes.WhiteSmoke; Ping_Value.Text = "Disconnected"; }

                Counter.Content = counter.ToString(); counter++;
            };

            BackWork_Pinger.RunWorkerAsync();
        }
    }
}

I wanna make a Pinger that works fine and not freezing my WinForm while disconnected or bad RoundTripTime. Is there any help to make a smooth pinger? or is there any demo project that i can modify from? Thank youuu :)

Answer 1

BackgroundWorker call DoWork on parrale thread and callback ProgressChanged on UI thread. In your code, the ping is done on ProgressChanged, thus in the UI thread. When the UI thread work, the UI is freezed.

To avoid freezing the screen, it is necessary to execute the work on another thread. The ping must be in DoWork :

public MainWindow()
{
    InitializeComponent();

    BackWork_Pinger.WorkerReportsProgress = true;

    BackWork_Pinger.DoWork += (_, args) =>
    {
        for (int i = 1; i > 0; ++i)
        {
            PingReply pingReply = null;
            if (IsConnectedToInternet())
            {
                try
                {
                    PingReply pingReply = pinger.Send("8.8.8.8");
                }
                catch
                { }
            }
            //Even if disconected
            //Report progress and pause
            BackWork_Pinger.ReportProgress(i, pingReply);
            Thread.Sleep(100);
        }
    };

    BackWork_Pinger.ProgressChanged += (_, args) =>
    {
        PingReply pingReply = pinger.Send("8.8.8.8");

        if (pingReply != null)
        {
            ...
        }
        else { Ping_Value.Foreground = Brushes.WhiteSmoke; Ping_Value.Text = "Disconnected"; } }
        Counter.Content = counter.ToString(); counter++;
    };

    // Start the work when the windows is loaded
    Loaded += (s, e) => { BackWork_Pinger.RunWorkerAsync(); };
    // Else, the work's ProgressChanged can update Ping_Value before it's intanciated
}

Answer 2

Your problem here is your wrong implementation. You are misunderstanding the Backgroundworker. Backgroundworker.DoWork is executed on a background thread while Backgroundworker.ProgressChanged is executed on the main tread/UI thread. You are currently executing the long-running operation on the UI thread which results in freezing this thread. The solution is to execute the long-running operation in the DoWork handler.

You should use the modern Task library to implement the background task using Task.Run and report progress using the Progress<T> class. This way you have a clean and modern async way to replace the old event based Backgroundworker.

In case of the Ping class you should know that it already exposes an asynchronous API which you should use.
Instead of Ping.Send call Ping.SendPingAsync. This way you don't need a background thread any more.
Also make use of the PingOptions and timeout to add more robustness to your application.

Notice that Ping implements IDisposable: you probably need to take care of the object lifetime and call Dispose().

Your improved cancelable version looks as followed:

private CancellationTokenSource CancellationTokenSource { get; set; }
private Ping PingSender { get; }

private void OnCancelButtonClicked(object sender, EventArgs e)
{
  // Abort
  this.CancellationTokenSource?.Cancel();
}

public async Task StartPingTargetAsync(string hostNameOrIp)
{  
  IProgress<(IPStatus PingStatus, long PingDuration)> progressReporter = new Progress<(IPStatus PingStatus, long PingDuration)>(ShowProgress);
  this.CancellationTokenSource = new CancellationTokenSource();
  CancellationToken cancellationToken = this.CancellationTokenSource.Token;

  try
  {  
    await PingIpAsync(hostNameOrIp, progressReporter, cancellationToken);
  }
  catch (OperationCanceledException)
  {
    // Optional: clean up/roll back
  }
  finally
  {
    this.CancellationTokenSource.Dispose();
    this.CancellationTokenSource = null;
  }
}

private async Task PingIpAsync(
  string hostNameOrIp, 
  IProgress<(IPStatus PingStatus, long PingDuration)> progressReporter,
  CancellationToken cancellationToken)
{
  while (true)
  {
    await Task.Delay(TimeSpan.FromMilliseconds(100), cancellationToken);
    cancellationToken.ThrowIfCancellationRequested();

    // Consider to use PingOptions and a time-out to add more robustness
    var options = new PingOptions (64, true);
    byte[] buffer = new byte[0];
    int timeoutInMilliseconds = (int) TimeSpan.FromSeconds(5).TotalMilliseconds;
    PingReply reply = await PingSender.SendPingAsync(hostNameOrIp, timeoutInMilliseconds, buffer, options);

    var progressData = (reply.Status, reply.RoundtripTime);
    progressReporter.Report(progressData);
  }
}

private void ShowProgress((IPStatus PingStatus, long PingDuration) progressData)
{
  Brush messageBrush = Brushes.WhiteSmoke;
  string message = $"{progressData.PingDuration} ms";

  switch (progressData.PingStatus)
  {
    case IPStatus.Success:
    {
      if (progressData.PingDuration < 100) 
      { 
        messageBrush = Brushes.Lime; 
      }
      else if (progressData.PingDuration < 300) 
      { 
        messageBrush = Brushes.Yellow; 
      } 
      else if (progressData.PingDuration < 500) 
      { 
        messageBrush = Brushes.DarkOrange; 
      }
      else 
      { 
        messageBrush = Brushes.Red; 
        message = "Bad"; 
      }
      break;
    }
    case IPStatus.TimedOut: 
      messageBrush = Brushes.WhiteSmoke; 
      message = "Disconnected"; 
      break;
  }

  Ping_Value.Foreground = messageBrush; 
  Ping_Value.Text = message; 
}