52

Would this be a proper way to dispose of a BackGroundWorker? I'm not sure if it is necesary to remove the events before calling .Dispose(). Also is calling .Dispose() inside the RunWorkerCompleted delegate ok to do?

public void RunProcessAsync(DateTime dumpDate)
{
    BackgroundWorker worker = new BackgroundWorker();
    worker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(worker_RunWorkerCompleted);
    worker.DoWork += new DoWorkEventHandler(worker_DoWork);
    worker.RunWorkerAsync(dumpDate);
}

void worker_DoWork(object sender, DoWorkEventArgs e)
{
    // Do Work here
}

void worker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    BackgroundWorker worker = sender as BackgroundWorker;
    worker.RunWorkerCompleted -= new RunWorkerCompletedEventHandler(worker_RunWorkerCompleted);
    worker.DoWork -= new DoWorkEventHandler(worker_DoWork);
    worker.Dispose();
}
galford13x
  • 2,483
  • 4
  • 30
  • 39
  • is this a background worker on a Form? – Paul Kohler Mar 30 '10 at 01:38
  • Yes it is, although I created the BGW programmatically rather than dropping it on the Form in the designer. As shown the BGW is created when I want to run the thread. The idea was to create a different BGW each time the thread was called and dispose of them when they are completed. – galford13x Mar 30 '10 at 21:31
  • I'm aware that this was aaaaaaages ago and it's the old way of registering handlers, however, this: -= new RunWorkerCompletedEventHandler(worker_RunWorkerCompleted) is creating a new delegate that wraps worker_RunWorkerCompleted, so it's not the same delegate as the first instance, and that first will still hang around, attached to the background worker. To ensure the handler is removed use: += worker_RunWorkerCompleted and -= worker_RunWorkerCompleted (without the 'new xxxEventHandler' wrapper. – Jono Stewart Nov 19 '19 at 13:15

5 Answers5

78

BackgroundWorker derives from Component. Component implements the IDisposable interface. That in turn makes BackgroundWorker inherit the Dispose() method.

Deriving from Component is a convenience for Windows Forms programmers, they can drop a BGW from the toolbox onto a form. Components in general are somewhat likely to have something to dispose. The Windows Forms designer takes care of this automatically, look in the Designer.cs file for a Form for the "components" field. Its auto-generated Dispose() method calls the Dispose() method for all components.

However, BackgroundWorker doesn't actually have any member that requires disposing. It doesn't override Dispose(). Its base implementation, Component.Dispose(), only makes sure that the component is removed from the "components" collection. And raise the Disposed event. But doesn't otherwise dispose anything.

Long story short: if you dropped a BGW on a form then everything is taken care of automatically, you don't have to help. If you didn't drop it on a form then it isn't an element in a components collection and nothing needs to be done.

You don't have to call Dispose().

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • 8
    Personally I do like to follow the policy of calling `Dispose` if present in the event that the implementation of the class actually changes... – Paul Kohler Mar 30 '10 at 02:04
  • 4
    I can't argue with that. But prefer always thinking: "what kind of object could be wrapped by a class that would require disposing?" And have a look. I have trouble writing code that makes no sense and don't buy into the notion that it will make sense some day. It works the other way around too: the Thread class really has disposable objects but doesn't implement IDisposable. To each his own. – Hans Passant Mar 30 '10 at 02:18
  • 1
    Just so I understand. A BGW doesn't need to be disposed since it has nothing to Dispose? I had read sometime that if events are not removed they can continue to hang out preventing freed resources when an object that relies on them are disposed. Is this never the case? – galford13x Mar 30 '10 at 19:24
  • @galford: it is technically possible for an event handler wired to a BGW's event to prevent a form object from being garbage collected. That never happens in practice because you always make the BGW object a member of the form. They'll get both garbage collected at the same time. – Hans Passant Mar 30 '10 at 19:28
  • I see, then my current implementation is lacking as I need to add this.Components.Add(BGW), and also make the BGW a persistant object within the form rather than createing the BGW each time the RunProcessAsync method is called. That makes sense. The BGW isn't memory intensive anyway, and recreating it each time just uses more cpu than is needed. – galford13x Mar 30 '10 at 21:28
13

Late to the game, but I just ran across a scenario related to your question that I thought I would share. If you create your worker at the class level and reuse it on successive operations without closing the app, if you don't remove the events after completion they will increment and run multiple times on each successive execution.

worker.RunWorkerCompleted -= new RunWorkerCompletedEventHandler(worker_RunWorkerCompleted);
worker.DoWork -= new DoWorkEventHandler(worker_DoWork);

Without the above my DoWork fires once the first time, twice the second time, etc. This is probably a no-brainer for most, but it took me a bit to figure it out, so hopefully this will help someone else out.

Paul
  • 441
  • 7
  • 14
  • 1
    Could you provide a more detailed code of your scenario. i use BW and this (as far as i know) never happened to me – Luis Filipe Aug 14 '12 at 12:58
  • Without writing up a complete example there's not much more to explain, but I'll try. if "worker" is defined outside of the methods that use it (i.e. globally at the application level), but you subscribe that worker within one of those methods without removing said subscription on each iteration it's subscriptions will continue to grow exponentially. – Paul Aug 14 '12 at 13:31
  • My understanding is that you are subscribing the events in a method that is called several times. If so: yes, of course; If not, i still do not grasp it. – Luis Filipe Aug 14 '12 at 13:54
  • That is exactly what I'm saying. Just stating an obvious observation from a over a year ago while discovering the usage of a BackgroundWorker. – Paul Aug 14 '12 at 14:50
  • course it is fireing multiple times you`re adding the body and complete multiple times – Mark Homer Dec 11 '13 at 17:04
  • @Mark Very helpful, glad you could contribute. – Paul Dec 11 '13 at 20:29
2

worker.Dispose() is not required, because Dispose() is automatically called. But before disposing the object you need to remove all events handlers.

This article informs us about this.

worker.RunWorkerCompleted -= new RunWorkerCompletedEventHandle(worker_RunWorkerCompleted);
worker.DoWork -= new DoWorkEventHandler(worker_DoWork);
Em1
  • 1,077
  • 18
  • 38
AlexN
  • 21
  • 1
  • 1
    If it was once true that the article mentioned removing event handlers, it is no longer so. In all versions of the linked article, no mention is made of events. – kbrimington Feb 11 '15 at 04:36
  • 1
    automatically called by who? as far as I am aware, also in the link you reference, it's up to the programmer to call Dispose. The only case it's guaranteed to be called is when using `using` keyword. – peval27 Sep 19 '18 at 07:16
0

Yes, this appears proper. Of course, disposable objects are better handled with using blocks, but you don't have that option here.

I usually create my background handers with form lifetimes, reuse them, and let the designer code handle disposal on form close. Less to think about.

Michael Petrotta
  • 59,888
  • 27
  • 145
  • 179
  • That is how I have always done it in the past. Although I did not realize that dropping a BackGroundWorker on a WinForm during design time added the BGW to the list of object that would be Disposed when the Form is disposed. I generally had created the BGW programmatically. – galford13x Mar 30 '10 at 19:21
0

If it's on a "WinForms" Form let the container take care of it (see the generated Dispose code in the Form.Designer.xyz file)

In practice I have found that you may need to create an instance of the container and add the worker (or other companent) to it, if anyone knows a more official way to do this yell out!!

PK :-)

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();

        // watch the disposed event....
        backgroundWorker1.Disposed += new EventHandler(backgroundWorker1_Disposed);

        // try with and without the following lines
        components = new Container();
        components.Add(backgroundWorker1);
    }

    void backgroundWorker1_Disposed(object sender, EventArgs e)
    {
        Debug.WriteLine("backgroundWorker1_Disposed");
    }

//... from the Designer.xyz file ...

    /// <summary>
    /// Clean up any resources being used.
    /// </summary>
    /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
    protected override void Dispose(bool disposing)
    {
        if (disposing && (components != null))
        {
            components.Dispose();
        }
        base.Dispose(disposing);
    }

}
Paul Kohler
  • 2,684
  • 18
  • 31