CODEWITHSUNDEEP
c#formsbackgroundworker
Thomton
Thomton

Reputation: 112

Background Worker RunWorkerCompleted isn't being called after DoWork finished

I'm creating a simple program that pings all the servers on our network and returns whether the ping requests were successful.

I'm trying to utilise background workers so that the user can press the ping button and the pings run in the background while they can do other things on the UI

DoWork runs fine, there's no loop to keep it there infinitely, and it reaches the line:

r = pinger.Send(s)

and then from my understanding it ends and so the RunWorkCompleted method should be called?

I'm relearning programming after a long abscense so if I missed something obvious I apologise ...

   public Form1()
    {

        InitializeComponent();
        backgroundWorker1.WorkerReportsProgress = true;
        backgroundWorker1.WorkerSupportsCancellation = true;
        backgroundWorker1.DoWork += new DoWorkEventHandler(backgroundWorker1_DoWork);
    }

    private void Ping_Btn_Click(object sender, EventArgs e)
    {

        count = Convert.ToInt32(pingSeconds_TxtBox.Text);

        if (backgroundWorker1.IsBusy != true)
        {
            // Start operation
            backgroundWorker1.RunWorkerAsync();
        }
       
    }

    private static void OnTimedEvent(Object source, ElapsedEventArgs e)
    {
        BackgroundWorker worker = new BackgroundWorker();
        worker.WorkerSupportsCancellation = true;
    }

    private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
    {
        BackgroundWorker worker = sender as BackgroundWorker;

        if(worker.CancellationPending == true)
        {
            e.Cancel = true;
        }
        else
        {
            for(int i = 0; i <= count; i++)
            {
                MessageBox.Show("something is happening");
                // Create ping object
                Ping pinger = new Ping();
                PingReply r;
                // IP to test ping
                string s = "###";

                try
                {
                    r = pinger.Send(s);

                }
                catch (Exception b)
                {
                    MessageBox.Show(b.ToString());
                }
            }
        }
    }

    private void backgroundWorker1_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {
        MessageBox.Show("Show me something");

        if(e.Cancelled == true)
        {
            statusLbl1.Text = "Cancelled";
        } else if(e.Error != null)
        {
            statusLbl1.Text = "Error: " + e.Error.Message;
        } else
        {
            statusLbl1.Text = "YEEEEEEEET";
        }
    }

...

Upvotes: 2

Views: 851

Answers (3)

Charlieface
Charlieface

Reputation: 71544

I strongly suggest you convert this code to use async await which is much better at representing the flow of code control, rather than using the old BackgroundWorker which is basically deprecated.

Note the following:

  • The main event handler should be async void but all other async functions should be async Task.
  • Use of SemaphoreSlim.WaitAsync(0) to check if we are busy.
  • Ping object needs a using or finally to dispose it, as does the CancellationTokenSource.
  • <= count looks like it should be < count because you begin at 0.
SemaphoreSlim sem = new SemaphoreSlim(1, 1);
CancellationToken token;

private async void Ping_Btn_Click(object sender, EventArgs e)
{
    if (!await sem.WaitAsync(0))
        return;

    var tokenSource = new CancellationTokenSource();
    try
    {
        var count = Convert.ToInt32(pingSeconds_TxtBox.Text);
        await RunPingsAsync(count, tokenSource.Token);
        statusLbl1.Text = "YEEEEEEEET";
    }
    catch (OperationCanceledException)
    {
        statusLbl1.Text = "Cancelled";
    }
    catch (Exception e)
    {
        statusLbl1.Text = "Error: " + e.Error.Message;
    }
    finally
    {
        sem.Release();
        tokenSource.Dispose();
    }
        
    MessageBox.Show("Show me something");
}

private Task RunPingsAsync(int count, CancellationToken token)
{
    for(int i = 0; i < count; i++)
    {
        token.ThrowIfCancellationRequested();

        MessageBox.Show("something is happening");
        // IP to test ping
        string s = "###";

        // Create ping object
        using (Ping pinger = new Ping())
        {
            var r = await pinger.SendPingAsync(s);
        }
    }
}

Upvotes: 2

Theodor Zoulias
Theodor Zoulias

Reputation: 43515

You need to attach your backgroundWorker1_RunWorkerCompleted event handler to the RunWorkerCompleted event. The C# compiler doesn't hook handlers to events based on naming conventions. You have to do it explicitly.

public Form1()
{
    InitializeComponent();
    backgroundWorker1.WorkerReportsProgress = true;
    backgroundWorker1.WorkerSupportsCancellation = true;
    backgroundWorker1.DoWork += backgroundWorker1_DoWork;
    backgroundWorker1.RunWorkerCompleted += backgroundWorker1_RunWorkerCompleted;
}

Upvotes: 2

Pankaj
Pankaj

Reputation: 2744

If you want to keep an infinite loop, then you have to make a loop in your backgroundWorker1_DoWork Method. Something like this

private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
{
    BackgroundWorker worker = (BackgroundWorker) sender;
    while (!worker.CancellationPending)
    {
        //Do your stuff here
        for(int i = 0; i <= count; i++)
        {
            MessageBox.Show("something is happening");
            // Create ping object
            Ping pinger = new Ping();
            PingReply r;
            // IP to test ping
            string s = "###";

            try
            {
                r = pinger.Send(s);

            }
            catch (Exception b)
            {
                MessageBox.Show(b.ToString());
            }
        }
    }        
 }

Also, it is not a good idea to display message boxes from your background thread, Log it in console or any file.

Upvotes: 0

Related Questions