Clear controls does not dispose them - what is the risk?

Question

There are multiple threads(a, b, c etc.) about the fact that Clear() ing items in the .NET component containers does not Dispose them(by calling Dispose(true).

Most frequently, IMHO, the Clear-ed components are not used anymore in the application, so it needs explicitly be Disposed after Clearing them from the parent containers.

Maybe is a good idea that collection's Clear method had a bool parameter dispose that when in true also disposes the collection elements before its removing from the list?

Darrickdarrill

If there's a call to Dispose in the finalizer, they will get disposed. If there's not, then there's probably (if the programmer followed accepted practices) nothing unmanaged to dispose and it's safe to just garbage collect them. – Hygrophilous

@Aviad: The problem that the Dispose method will never be called by the GC, so you need to do it by yourself, before calling Clear on collection. – Darrickdarrill

What I mean is, if the control's programmer did not put a Dispose call in its finalizer (which is called when the object is garbage collected) - then there's probably nothing to dispose. – Hygrophilous

See also Hans's detailed description of the problems here: https://mcmap.net/q/590393/-removing-dynamically-created-controls-in-c – Sublimation

Answer 1

Asking for modifications like this is pointless, the Windows Forms team has been disbanded quite a while ago. It is in maintenance mode, only security issues and OS incompatibilities are considered.

It is otherwise simple enough to create your own method to do this:

  public static class ExtensionMethods {
    public static void Clear(this Control.ControlCollection controls, bool dispose) {
      for (int ix = controls.Count - 1; ix >= 0; --ix) {
        if (dispose) controls[ix].Dispose();
        else controls.RemoveAt(ix);
      }
    }
  }

Now you can write:

  panel1.Controls.Clear(true);

Clinic

IIRC, when you Dispose a Control, it automatically removes the control from the corresponding ControlCollection, so you actually don't need the RemoveAt (and might end up with an IndexOutOfRangeException). – Hopeless

yeah... at least for .NET 2 this will not work. But the question is a little bit other. Is there a "risk" do calling "Clear" without Dispose? – Darrickdarrill

Of course, you'll leak the controls. Wasn't that obvious from the other threads? – Clinic

@nobugs: yes, now, in other words, you say that, normally, calling Clear without Dispose has no sense; Microsoft will not change the code, so all we have to do is remember that, use these methods together for .NET < 3.5 and maybe also implement extension methods in .NET >= 3.5 – Darrickdarrill

Erm, I didn't say that. In fact, I don't think Clear() makes a lot of sense in general. WF was optimized to use UserControls and the designer. Container controls already know how to properly dispose their child controls. – Clinic

@nobugs: I think Clear it's a classic member of any collection(not only the component one), this should be its sense. Container controls "already know how to properly dispose"? What do you mean? As we can see this is not the "Clear" case. Probably they at least can add an "obsolete" attribute on the controls containers "Clear" method. – Darrickdarrill

Disposing a control automatically disposes any child control in its Controls collection. No help is needed. – Clinic

@nobugs: Yes, you are right. BUT, after using CLEAR the parent will have any control in its controls collection, so all the controls that was Clear ed will remain un Dispose d, when the parent control is disposed. – Darrickdarrill

@serhio: Are you sure that Controls.Clear() isn't overridden in ? I think it handles the dispose as Hans is pointing out. – Infracostal

Yes, Control.Dispose() will automatically remove the control from the parent ControlCollection, see source.dot.net/#System.Windows.Forms/System/Windows/Forms/… – Pother

Answer 2

Answering the "what is the risk" question, the risk (or a risk) is running out of window handles, although it can take a while.

I have a "window designer" that generates a window from a script. Each time I change the script, the window is rebuilt (the controls cleared and readded). With a particularly complex window, and using Controls.Clear() each time, after many dozens of refreshes, I will eventually get a "no more window handles" exception and not be able to create any more controls.

Easy enough to replace the Controls.Clear() call with something like:

Controls.Cast<Control>().ForEach(c => c.Dispose());

Gentilis

Answer 3

@Hans Passant answer is good but in case of asynchronous programming you should consider to remove the object before dispose it to avoid some thread to iterate over a disposed object.

More or less something like this:

public static class ExtensionMethods {
  public static void Clear(this Control.ControlCollection controls, bool dispose) {
    for (int ix = controls.Count - 1; ix >= 0; --ix) {
      var tmpObj = controls[ix];
      controls.RemoveAt(ix);
      if (dispose) tmpObj.Dispose();
    }
  }
}

Murton

Control.Dispose() will automatically remove the control from the parent ControlCollection, see dot.net-source. An additional RemoveAt would probably lead to an IndexOutOfRangeException. – Pother