11

If I have a method like this:

public void Show()
{
   Form1 f = new Form1();
   f.ShowDialog();
}

Do I still need to call dispose on the form even though it will go out of scope, which will be eligible for garbage collection.

From some testing, calling this Show() multiple times .. at some point it seems like the GC collects it since I can see the memory spiking then it goes down at some point in time.

From MSDN it seems to say you MUST call dispose when the form is not needed anymore.

pdiddy
  • 6,217
  • 10
  • 50
  • 111

7 Answers7

16

What tends to happen is if the item has purely managed resources, calling dispose is not necessarily required, but is strongly advised because it makes disposal deterministic. It isn't always required (in a technical sense) because those managed resources would likely themselves now be eligible for GC, or there is actually nothing to dispose by default and it's an extensibility point.

For unmanaged resources, the Dispose Pattern advises implementing a finalizer, which will be called on GC. If types do not implement the finalizer and dispose is not called, then it is possible (well, very likely) that resources would be left unhandled. Finalizers are the last chance offered by the runtime for clearing your stuff up - they are also time-limited.

Note, that it does not make GC or managed memory reclamation deterministic, disposal is not delete from C++. A disposed item could be a long way away from actually being collected. However, in the managed world, you don't care about deterministic collection, only resource management - in other words, disposal.

That said, I always make sure I call Dispose or use a using statement if a type is disposable, regardless of whether it uses managed or unmanaged resources - it is the expected convention:

public void Show()
{
    using (var f = new Form1())
    {
        f.ShowDialog();
    } // Disposal, even on exceptions or nested return statements, occurs here.
}

Update:

After a discussion with Servy I feel I have to express this point as the reasoning behind my advice of disposing where possible. In the case of MemoryStream, it is clearly a disposable type, but actually does not dispose of anything currently.

Relying on this, however, is to rely on the implementation of MemoryStream. Were this to change to include an unmanaged resource, this would then mean that a reliance on MemoryStream not having anything to dispose becomes problematic.

Where possible (as is the case with IDisposable) I prefer to rely on the public contract. Working against the contract in this instance would mean I am safe from underlying implementation changes.

Adam Houldsworth
  • 63,413
  • 11
  • 150
  • 187
  • The only cases I can think of in which production-quality code should not call `Dispose` on an object are those in which either (1) keeping track of the object's lifetime would be particularly onerous, and the object is known to be of a particular type which will clean itself up adequately even when not disposed; and/or (2) the object has a somewhat broken design which uses its `Dispose` method purely to unconditionally clean up a passed-in object (e.g. `StreamReader`), and the supplier of that passed-in object wishes to retain ownership. – supercat Jul 13 '12 at 15:54
  • @supercat I can agree. I used to have a situation where the life-cycle was out of my control. I am just making the point that *by default* I try to dispose of everything. Of course, with all generalisations, there are exceptions. – Adam Houldsworth Jul 13 '12 at 15:57
  • may I ask: would it be different if the form would not be assigned to a pointer? Like `new Form().ShowDialog()` ? Or would it be even worse, since if I'm not keeping a pointer I'm not able to dispose it? (The Form in particular is only needed to calculate and display additional information on demand) – skofgar Apr 05 '13 at 15:10
  • 1
    @skofgar If you don't hold on to the reference the `Application.OpenForms` property will do. When all references are dropped, the GC will eventually dispose of it via the finalizer if it's implemented. In the case of a `Form`, not keeping hold of the reference isn't too big a deal. – Adam Houldsworth Apr 08 '13 at 14:33
4

Although you rarely have to manually dispose in C# imo, you could try it like this:

    public void Show()
    {
       using (Form1 f = new Form1())
       {
         f.ShowDialog();
       }
    }

Then at the last accolade of the using part it will get disposed of automatically.

Gerald Versluis
  • 30,492
  • 6
  • 73
  • 100
4

ShowDialog has side effect of keeping the GDI objects alive. In order to avoid GDI leak we need to dispose the ShowDialog appropriately. Where as Show method does not have any implication and GDI will be released appropriately. It is recommended to dispose the showDialog and do not rely on Garbage collector.

Mendy
  • 147
  • 1
  • 9
3

You could simply do:

using (var f = new Form1())
   f.ShowDialog();
Druid
  • 6,423
  • 4
  • 41
  • 56
2

If you want to explicitly dispose, use

 using(Form1 f = new Form1()){

            f.ShowDialog();
        }

This ensures Dispose() is called, as well as it occurs immediately

James
  • 2,445
  • 2
  • 25
  • 35
1

In your specific example, no, it's unlikely that it would be particularly useful. Forms do not hold onto a significant amount of resources, so if it takes a little bit longer for some portion of it's code to get cleaned up it isn't going to cause a problem. If that form just happens to be holding onto a control that is used to, say, play a video, then maybe it's actually holding onto some significant number of resources, and if you actually do dispose of those resources in the dispose method then it's worth taking the time to call dispose. For 99% of your forms though, their Dispose method will be empty, and whether you call it or not is unlikely to have any (or any noticeable) effect on your program.

The reason that it's there is primarily to enable the ability to dispose of resources in those 1% of cases where it's important.

It's also worth noting that when a Form is closed its Dispose method is already being called. You would only ever need to add a using or explicit Dispose call if you want to dispose of a Forms resources before that form is closed. (That sounds like a generally bad idea to me). This is easy enough to test. Just create a project with two forms. Have the second form attach an event handler to the Disposing event and show a message box or something. Then when you create an instance of that form and show it (as a dialog or not) you'll see that when you close it the message box will pop up right away, even if you keep the 'Form' instance around and without you ever needing to add a using or Dispose call.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • Thanks exactly what I was looking for as anwswer – pdiddy Jul 12 '12 at 15:13
  • 1
    @pdiddy Until someone comes along and places something in `Form1` that needs disposing, but because no one is calling it never gets disposed deterministically. – Adam Houldsworth Jul 12 '12 at 15:15
  • @AdamHouldsworth And yet the vast majority of forms are only ever instantiated once, ever, so it's not exactly a big chore to track that one place down and add a dispose call. It's also worth noting that many forms aren't shown via a dialog, and when it's not you can't just wrap it in a `using` and be done with it. Determining when to dispose of it is much harder. – Servy Jul 12 '12 at 15:17
  • 1
    @Servy That assumes they are instantiated directly and not by some other means such as reflection. I agree that most of the time there is no functional difference, but as an act of getting into the habit, it is worth using `using` anyway - it's not exactly like there is a worthwhile performance benefit of not using `using` if there is only the one call. And it also assumes that a person amending the `Dispose` method will take it upon themselves to check it is disposed everywhere and not assume it is because of the accepted convention of `IDisposable`. I wouldn't check by default. – Adam Houldsworth Jul 12 '12 at 15:19
  • @AdamHouldsworth If you're not showing it as a dialog sure there is, since you won't be able to use a using at all in that case. You'll need more complex logic to actually dispose of it whenever it will no longer be used (which isn't always easy to determine). As for conventions; out of the code that I look through involving forms, I virtually never see anyone disposing of forms. If I made a form that relied on being disposed I would take the time to ensure it was actually being disposed because I know most people don't dispose of them. It's the same as for `DataTable`s. – Servy Jul 12 '12 at 15:20
  • 1
    @Servy I don't understand what you are getting at with the dialog bit... in the OPs example it is shown as a dialog and is still usable once it has been closed and you can easily wrap it in a dispose call... – Adam Houldsworth Jul 12 '12 at 15:21
  • @AdamHouldsworth Yes, in this one case it's shown as a dialog, but you're saying that you should always and forever wrap forms in a `using`. That won't work the second you aren't showing it as a dialog. – Servy Jul 12 '12 at 15:22
  • 1
    @Servy Fine, I wasn't very clear when I said that. What I was *implying* is that it is a good idea to dispose items that require disposing of, `using` is just an easy way to do it most of the time. I disagree with your sentiments on the forms, but I suppose it would be open to debate as to who's fault it would be for introducing the memory leak. Also, for completeness, you can have a form that isn't shown as dialog in a `using`, but you wouldn't be able to show it for very long before it gets disposed lol – Adam Houldsworth Jul 12 '12 at 15:25
  • For example, I dispose of `MemoryStream` even though it currently doesn't need to dispose of anything. However, that is then a reliance on the **implementation**, not on the **public contract**. I stick to the public contract and `Dispose` it even though it is *technically* pointless. If it changes in future, I'm safe. This is the core of my point. – Adam Houldsworth Jul 12 '12 at 15:27
  • @AdamHouldsworth Yes, disposing of every form would make them much more future proof, but it's not worthwhile to future proof everything. If you don't need it know, and don't forsee needing it soon, you're spending a fairly significant development effort on something that you're very unlikely to ever use. Development time that could be spent on all sorts of other more important tasks. If you're just wrapping a few lines in a `using` it's not a huge effort, but as soon as you want to handle any non-dialog forms that effort will become *significant*. – Servy Jul 12 '12 at 15:34
  • 1
    @Servy "Significant" is too generalised to make a point on, in my opinion. Looking after the life-cycle of forms is not difficult considering we have the `Closed` event. I disagree that it adds to the development cost enough to make my default approach to "dispose nothing" as opposed to "default dispose". In my opinion, the very small cost of disposing stuff is enough to warrant doing it by default considering that it future proofs me against potential memory leaks - a very costly debugging session. – Adam Houldsworth Jul 12 '12 at 15:36
  • 1
    @pdiddy Anyway, the reason I originally commented was to make the point that although Servy is perfectly correct in his statement that most of the time you are safe, it doesn't actually explain that a previously safe form can become unsafe without you knowing about it. It was just a heads up that things can change underneath your current implementations. – Adam Houldsworth Jul 12 '12 at 15:42
  • @AdamHouldsworth See my edit. `Form` already calls `Dispose` when a form is closed; you don't need to explicitly add *another* event handler to call dispose in the `Closing` event. – Servy Jul 12 '12 at 15:59
  • @Servy Fair enough, but that is another example of trusting the implementation rather than the public contract. I might in future not care too much about form disposal, but my personal approach to disposable items is to call it when I'm done. – Adam Houldsworth Jul 12 '12 at 16:01
  • @Servy Actually to your point I'd trust this even less, as when the form is closed and it apparently disposes - you can happily re-open it and it will work. Likely this is down to it's implementation. That said, if your code were disposing the item, it expresses the intent of the code better than it happening silently. The next developer who comes along can see when it is disposed and code accordingly, as opposed to guessing about disposal. – Adam Houldsworth Jul 12 '12 at 16:09
  • 1
    It is very dangerous to assume that one may safely abandon a `Form` without disposing it. In many cases, one can get away with such things, but if a form subscribes to an event from a long-lived object, failure to `Dispose` the form may prevent the form, or anything to which it holds a reference, from becoming eligible for collection until such time as the long-lived object is disposed. – supercat Jul 13 '12 at 15:47
  • @Servy, "And yet the vast majority of forms are only ever instantiated once" — WAT??! – Sasha Mar 16 '17 at 13:43
  • 2
    @Servy, "`Form` already calls `Dispose` when a form is closed" — that [seems](http://stackoverflow.com/a/3097383/1421194) to be **not true**: "During that process, *if the form was not shown modally*, Dispose is called on the form. […] However, if you do form1.ShowDialog() to show the form modally, the form *will not be disposed, and you'll need to call form1.Dispose() yourself*." – Sasha Mar 16 '17 at 13:45
-1

Yes, you need and MUST call Dispose or use using statement, otherwise it can be, that instance remains in memory.

You can check it, if you put a Timer to the form and set a break point in the Timer.Tick handler. Even after closing the form without Dispose the handler will be called.

Rekshino
  • 6,954
  • 2
  • 19
  • 44