0

UPDATE:

I'm sorry but I'm confused. It could be that the confusion comes from old perceptions on GC/Dispose vs. new perception.

If you look here in this (albeit no longer maintained) official Microsoft Documentations, it indicates that Dispose patterns are also used for releasing managed resources:

protected override void Dispose(bool disposing)
{
    if (!disposed)
    {
        if (disposing)
        {
            // Release **managed** resources. (!!!!!)
        }
        // Release unmanaged resources.
        // Set large fields to null.
       // Call Dispose on your base class.
        disposed = true;
    }
    base.Dispose(disposing);
}

This is contrary to what is written in the new version, which states:

Managed memory (memory allocated using the C# operator new) does not need to be explicitly released. It is released automatically by the garbage collector (GC).

Maybe it's just a mistake in the documentation, that threw me off (damn you Microsoft). But maybe - it could be that in the early days of the GC, it was not so trusted, and the guidelines were "you can clean yourself, if you know what you're doing". And now days it's so trusted, that the guidelines are "on managed resources don't ever use, we know better than you". If it's not a mistake - then there's clearly some change or shift in perception here.

As for the question - I won't do anything and will let GC do what it knows best.


Old Question:

I have a TreeView ViewModel class that has nodes in it, which are recursive classes (they can contain themselves).

e.g.

class RootViewModel // root
{
   List<ItemViewModel> Children = new List<ItemViewModel>();
}

class ItemViewModel // node
{
    List<ItemViewModel> Children = new List<ItemViewModel>();
}

At some point all the data from the TreeView is transferred/saved to other objects, and I don't need the TreeView anymore. I'm guessing I need to dispose of it ? (while the window containing the ViewModel is closed, the ViewModel class object is saved in a static variable).

Do I need to dispose every node by itself (recursive dispose) or is it enough to set the root object to null and call GC ?

I should note that my disposing doesn't have any unmanaged resources, I'm just setting all children in the Children-lists to null for every node.

So again -

  1. Option A: Set the static object to null, call GC.Collect().
  2. Option B: Recursively set all nodes to null, set static object to null, call GC.Collect(). - Seems to be out of the question according to answers and comments so far
  3. Option C: GC sees you stopped using the static object, so it will dispose of it by itself.
Maverick Meerkat
  • 5,737
  • 3
  • 47
  • 66
  • 3
    If your solution includes the requirement of forcing a garbage collection then you are doing something very, very wrong. Also you seem to have some false beliefs about the relationship between disposables and garbage collection; remember, disposing is to free resources that are **not garbage collected**. None of your options are the correct thing to do. – Eric Lippert Sep 03 '17 at 15:12
  • What problem do you actually have that you are trying to solve? – Eric Lippert Sep 03 '17 at 15:17
  • @EricLippert - I want to make my app efficient and optimized. I just closed a window, and there are a lot of objects used by that window VM. I want to make sure they are freed. – Maverick Meerkat Sep 03 '17 at 15:26
  • 1
    Then you need to do some *user focused* measurements. What problem are your *users* having? Once you can measure that problem then you can do experiments and discover whether your changes are empirically improving or degrading the user experience. I note that causing garbage collections often degrade the user experience because they can make applications unresponsive. If you don't have a problem that impacts users, then spend your valuable time on something other than "fixing" problems no one has. – Eric Lippert Sep 03 '17 at 15:29
  • You say you want to be "efficient". Efficiency is defined as value produced divided by work expended; how are you measuring value and work? If you're unable to measure these things then you are unable to optimize for efficiency. – Eric Lippert Sep 03 '17 at 15:31
  • I'm trying to anticipate problems before they occur, and also learn something along the way, to improve my understanding and skills. There seems to be some contradicting information here, as your categorical rejection of me setting the static object to null and calling GC.Collect() seems in contrary to https://stackoverflow.com/a/478177/6296435 and https://stackoverflow.com/a/27801948/6296435 and partially https://stackoverflow.com/a/2926890/6296435 ("Sometimes, you may need to set an object to null in order to make it go out of scope...") – Maverick Meerkat Sep 03 '17 at 15:40
  • 1
    Storing your model in the static variable seems to be a bad idea that caused the problem you are trying to solve. Could you avoid storing it in the static variable? E.g. store it in your ViewForm, so when the form is closed, the model will be collected by GC. – Sergey L Sep 03 '17 at 19:58
  • I can´t see any text within the old docs that indicate you should call `Dispose` on managed ressources also. – MakePeaceGreatAgain Sep 04 '17 at 14:07
  • @HimBromBeere " // Release managed resources" – Maverick Meerkat Sep 04 '17 at 14:48
  • I suppose this just means that you should call `Dispose` for all your managed dependencies also if they implement it. The object itself might be managed, however it may hold unmanaged ressources that need to be released. So in order to also release the resources of your referenced objects, call `Dispose` on them also. – MakePeaceGreatAgain Sep 05 '17 at 09:59
  • hmm... ok, that makes sense... – Maverick Meerkat Sep 05 '17 at 10:51
  • 1
    Also see the comment from Brian under my answer. – MakePeaceGreatAgain Sep 05 '17 at 15:19

1 Answers1

1

You should Dispose to release unmanaged ressources only, e.g. some file-stream. So when you don´t have any unmanaged resources, no need to use Dispose at all. See the docs:

Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.

Instead just rely on the GC to release all instances as soon as there are no more references to them existing. That is when there´s no variable pointing to it. So in the moment the parent node is released by the GC also its children are (assuming you don´t have any other variables pointing to them). However calling GC.Collect is almost allways a bad idea, GC is usually doing a good job and you shouldn´t investigate.

Setting your variables to null will however not mark them for garbage-collection unless you have some really big code-block (e.g. one single method containing thousands of lines). In this case your variables may not get out of scope for a long time. Doing this is therefor more a sign of a code-smell, you should devide your code into smaller parts that do one single thing.

MakePeaceGreatAgain
  • 35,491
  • 6
  • 60
  • 111
  • As I mentioned - I have a static variable holding the root node - is it considered a reference? Do I need to set it to null ? – Maverick Meerkat Sep 03 '17 at 14:44
  • 1
    @DavidRefaeli In case of a static variable you can set it to null or rely on the GC to release it as soon as your app terminates. – MakePeaceGreatAgain Sep 03 '17 at 14:49
  • 1
    *rely on the GC to release all instances as soon as there are no more references to them existing. That is when there´s no variable pointing to it* Nitpicking, I know, but somewhat relevant to the question at hand. The rule is no *reachable* reference; an object with references pointing to it can be collected if they are not reachable. This enables de the GC to collect objects that reference each other but are not reachable. – InBetween Sep 03 '17 at 18:28
  • 2
    Just a little nitpicking, but a FileStream is a _managed_ resource. Application programmers almost never have to deal with unmanaged resources. – H H Sep 04 '17 at 11:00
  • 3
    @HenkHolterman: `FileStream` implements `IDisposable`. So, any class which makes use of a `FileStream` as an instance property (rather than as local wrapped in a `using` block) should still implement `IDisposable` (but will not need to implement a finalizer). – Brian Sep 05 '17 at 15:14
  • 1
    Yes, yes and yes. Not so clear why you think you need to tell me. – H H Sep 05 '17 at 16:34