Winforms: Control can't be properly GC-collected after disposing because of it referenced via the System.Windows.Forms.Control.cachedLayoutEventArgs field

[DmitryGaravsky]

The same bug is reproducible with .NET Framework but it seems it has no chance to be ever fixed.
But the .NET Core looks like a good place for introducing some improvements in this area.

Problem description

Each instance of the System.Windows.Forms.Control type contains a private member variable of the LayoutEventArgs type - cachedLayoutEventArgs . And, the LayoutEventArgs instance typically contains a reference to some specific control.
Sometimes, the cachedLayoutEventArgs field is not cleared when the child control disposing of does not affect the layout process of the parent control due to some reasons.

Reproduction

Here are the minimal reproduction-steps:

1. Create a form with two buttons.
2. Add the following code for the corresponding Button.Click handlers:
   c# void btnOpenView_Click(object sender, System.EventArgs e) { var view = new Panel() { BackColor = Color.Red }; view.Name = "View"; view.Bounds = new Rectangle(100, 100, 100, 100); view.Parent = this; } void btnCloseView_Click(object sender, System.EventArgs e) { SuspendLayout(); var view = this.Controls.Find("View", false)[0]; if(view != null) view.Dispose(); ResumeLayout(false); }
3. Press the "Open View" button - red panel appears.
4. Press the "Close View" button - red panel disappears.

Actual behavior:
The panel is still referencing via the System.Windows.Forms.Control.cachedLayoutEventArgs field at the form level. As result, it can't be GC-collected properly and still in memory.

Expected behavior:
The panel should not be referenced.

Proposal for Fix

It looks like we should use the WeakReference when caching the LayoutEventArgs.
As an alternative solution, we can validate the cachedLayoutEventArgs value on child control removing and clear the problematical field.

[zsd4yr]

Sometimes, the cachedLayoutEventArgs field is not cleared when the child control disposing does not affect the layout process of the parent control due to some reasons.

Could you be more clear about these scenarios? What is not triggering when the child control disposing does not affect the layout process of the parent control?

[DmitryGaravsky]

Hi, @zsd4yr

Could you be more clear about these scenarios?

From my experience, there are two cases when this issue is 100% reproducible:

• manual locking of layout processing(already demonstrated in the Reproduction section);
• removing a control from an invisible container (inactive tab-page for example);

Here are the steps for last case reproduction:

1. Create a form with two buttons and TabControl with two pages.
2. Add the following code for the corresponding Button.Click handlers:

void btnAddViewToTab_Click(object sender, System.EventArgs e) {
    var view = new Panel();
    view.Name = "View";
    view.Dock = DockStyle.Fill;
    view.Parent = tabPage2;
}
void btnRemoveViewFromTab_Click(object sender, System.EventArgs e) {
    var view = tabPage2.Controls.Find("View", false)[0];
    if(view != null)
        view.Dispose();
}

1. Press the "Add View to Tab" button.
2. Press the "Remove View from Tab" button.
   After this, the panel is still referenced via the System.Windows.Forms.Control.cachedLayoutEventArgs field at the tabPage2 level.

Also, there are some scenarios when this issue is reproducible from time to time:

• closing an MDI-child form within the MDI-parent form;
• removing the control from the controls hierarchy when it placed at the 12-15 level (x64 windows only).
  IMO, the last case is related to the known windows design limitation.

[KlausLoeffelmann]

I'll take a look!

[weltkante]

@KlausLoeffelmann @DmitryGaravsky

Proposal for Fix
It looks like we should use the WeakReference when caching the LayoutEventArgs.

I think this won't work, if nobody holds a reference to the LayoutEventArgs it will be collected immediately, making the cache useless. What you want is a ConditionalWeakTable where the key is the Control and the value the LayoutEventArgs (ConditionalWeakReference does not exist unfortunately, and the framework API required to build one is internal).

[DmitryGaravsky]

@weltkante

if nobody holds a reference to the LayoutEventArgs it will be collected immediately, making the cache useless.

Yes, you're absolutely correct, but I proposed not the exact replacement of LayoutEventArgs cachedEventArgs; with the WeakReference cachedEventArgs; but "something based on weak-references" that can resolve this issue.
As an alternative solution, we can validate the cachedLayoutEventArgs value on child control removing and clear the problematical field.

[zsd4yr]

@KlausLoeffelmann, we have been very busy these last few weeks! When you get a chance, could you update this thread with any progress?

[KlausLoeffelmann]

Sure. We should re-prioritize issues this week, anyway, but I am guessing, this will be waiting a little time longer.