C#: Do I need to dispose a BackgroundWorker created at runtime?

Question

I typically have code like this on a form:

    private void PerformLongRunningOperation()
    {
        BackgroundWorker worker = new BackgroundWorker();

        worker.DoWork += delegate
        {
            // perform long running operation here
        };

        worker.RunWorkerAsync();
    }

This means that I don't dispose the BackgroundWorker, whereas if I had added it by the form designer then I think it would get disposed.

Will this cause any problems? Is it more correct to declare a module-level _saveWorker, and then call Dispose on it from the form's dispose?

Answer 1

I wouldn't bother, the only resource the Bgw could hold on to is the Thread and if you don't have an infinite loop in your delegate then you are fine.

BackgroundWorker inherits IDisposable() from Component, but doesn't really need it.

Compare it to pushing your method directly on the ThreadPool. You don't (can't) Dispose threads, certaily not those from the Pool.

But if your sample is complete in the sense that you are not using the Completed event or Progress/Cancel features you could just as well use ThreadPool.QueueUserWorkItem().

Answer 2

Yes, you should dispose of the background worker.

You may find it easier to use ThreadPool.QueueUserWorkItem(...) which doesn't require any clean up afterwards.

---

Additional detail on why you should always call Dispose():

Although if you look in the BackgroundWorker class it doesn't actually do any thread clean up in it's dispose method, it is still important to call Dispose because of the effect the class has on the garbage collector.

Classes with finalizers are not GCed immediately. They are kept and added to the finalizer queue. The finalizer thread then runs, (which following the standard pattern calls dispose). This means the object will survive into GC generation 1. And gen 1 collections are far rarer than gen 0 collections, so you object sticks around in memory for much longer.

If however you call Dispose(), the object will not be added to the finalization queue, so is free to be garbage collected.

It's not really big problem, but if you are creating a lot of them you'll end up using more memory than necessary. It should really be considered good practise to always call dispose on objects that have a dispose method.

So I suppose, all in all, it's not a 100% hard and fast requirement. Your app won't explode (or even leak memory) if you don't call Dispose(), but in some circumstances it may have negative effects. The background worker was designed for use from as a WinForms component so use it that way, if you have different requirements and don't want to use it as a WinForms component, don't use it, use the correct tool for the job, like the ThreadPool.

Answer 3

The challenge is making sure you only dispose the BackgroundWorker after it's finished running. You can't do that in the Completed event, because that event is raised by the BackgroundWorker itself.

BackgroundWorker is really intended to be used as a component on a WinForms form, so I would recommend either doing that, or switching to something like Thread.QueueUserWorkItem. This will use a thread-pool thread, and won't require any special cleanup when it's finished.

Answer 4

It's considered a best practice to call Dispose() for all IDisposable objects. That allows them to release unmanaged resources they may be holding, such as Handles. IDisposable classes also should have finalizers, whose presence can delay the time at which the GC is allowed to fully collect those objects.

If your class allocates an IDisposable and assigns it to a member variable, then in general it should itself also be IDisposable.

However, if you don't call Dispose(), then the Finalizer will eventually be called, and the resources will eventually be cleaned up. The advantage of calling it explicitly is that those things can happen more quickly and with less overhead, which improve app performance and reduce memory pressure.

Answer 5

In my view, in general, if it's IDisposable, it should be Dispose()d when you're done with it. Even if the current implementation of BackgroundWorker doesn't technically need to be disposed, you don't want to be surprised by later internal implementations that might.

Answer 6

Why not wrap in a using statement? Not much extra effort and you get the disposal:

private void PerformLongRunningOperation()
    {
        using (BackgroundWorker worker = new BackgroundWorker())
        {
            worker.DoWork += delegate
                             {
                                 // perform long running operation here 
                             };
            worker.RunWorkerAsync();
        }
    }

EDIT:

Ok, I put together a little test to kinda see what is going on with disposal and whatnot:

using System;
using System.ComponentModel;
using System.Threading;

namespace BackgroundWorkerTest
{
    internal class Program
    {
        private static BackgroundWorker _privateWorker;

        private static void Main()
        {
            PrintThread("Main");
            _privateWorker = new BackgroundWorker();
            _privateWorker.DoWork += WorkerDoWork;
            _privateWorker.RunWorkerCompleted += WorkerRunWorkerCompleted;
            _privateWorker.Disposed += WorkerDisposed;
            _privateWorker.RunWorkerAsync();
            _privateWorker.Dispose();
            _privateWorker = null;

            using (var BW = new BackgroundWorker())
            {
                BW.DoWork += delegate
                                 {
                                     Thread.Sleep(2000);
                                     PrintThread("Using Worker Working");
                                 };
                BW.Disposed += delegate { PrintThread("Using Worker Disposed"); };
                BW.RunWorkerCompleted += delegate { PrintThread("Using Worker Completed"); };
                BW.RunWorkerAsync();
            }

            Console.ReadLine();
        }

        private static void WorkerDisposed(object sender, EventArgs e)
        {
            PrintThread("Private Worker Disposed");
        }

        private static void WorkerRunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
        {
            PrintThread("Private Worker Completed");
        }

        private static void WorkerDoWork(object sender, DoWorkEventArgs e)
        {
            Thread.Sleep(2000);
            PrintThread("Private Worker Working");
        }

        private static void PrintThread(string caller)
        {
            Console.WriteLine("{0} Thread: {1}", caller, Thread.CurrentThread.ManagedThreadId);
        }
    }
}

Here is the output:

Main Thread: 1
Private Worker Disposed Thread: 1
Using Worker Disposed Thread: 1
Private Worker Working Thread: 3
Using Worker Working Thread: 4
Using Worker Completed Thread: 4
Private Worker Completed Thread: 3

From some testing, it appears that Dispose() has basically no effect on an initiated BackgroundWorker. Whether or not you call it in the scope of a using statement, or use it declared in code and immediately dispose it and dereference it, it still runs normally. The Disposed Event occurs on the main thread, and the DoWork and RunWorkerCompleted occur on thread pool threads (whichever one is available when the event fires). I tried a case where I unregistered the RunWorkerCompleted event right after I called Dispose (so before DoWork had a chance to complete) and RunWorkerCompleted did not fire. This leads me to believe that you can still manipulate the BackgroundWorker object despite it being disposed.

So as others have mentioned, at this time, it does appear as though calling Dispose is not really required. However, I do not see any harm in doing it either, at least from my experience and these tests.