C#: Do I need to dispose a BackgroundWorker create

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 method?

Answer 1

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 2

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 3

Call dispose in your RunWorkerCompleted event.

BackgroundWorker wkr = new BackgroundWorker();
wkr.DoWork += (s, e) => {
    // Do long running task.
};
wkr.RunWorkerCompleted += (s, e) => {
    try {
        if (e.Error != null) {
            // Handle failure.
        }
    } finally {
        // Use wkr outer instead of casting.
        wkr.Dispose();
    }
};
wkr.RunWorkerAsync();

The extra try/finally is to ensure that Dispose gets called if your completion code raises an exception.

Answer 4

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, certainly 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 5

The completion handler is run on the original thread (i.e., not the background thread from the thread pool)! Your test results actually confirm that premise.

Answer 6

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.