Forms that won't Dispose

  • Thread starter Thread starter Alex Clark
  • Start date Start date
A

Alex Clark

Hi MS & All others,

I've been investigating a memory leak in my application which is proving
particularly hard to fix, and I came across something very unusual.

I've determined that the leak is caused by a particular form that won't
"dispose". I'm using a program called ".NET Memory Profiler" to check to
see whether there's still an instance of my form even after it has been
closed & disposed.

After doing some testing, I've now come across a very unusual situation: In
a simple little app, I can load Form2 from within Form1, display it using
ShowDialog, close it (and then call Dispose on it from within Form1) and the
memory profiler reports that there are 0 instances of Form2 currently
running, just as it should be.

However, if I have a context menu on Form2 and I display it (handling
Form2's click event and showing the menu from code), and then close &
dispose it, it remains in the list of instantiated classes. I have tried
this several times, and was able to end up with a list of over a dozen
instances of Form2 despite the fact they had been closed and disposed of.
This ONLY ever happened if I'd clicked to show the Context-Menu, if I didn't
do that then Form2 would happily Dispose.

Is there some quirk where extra code is needed to dispose of a Form after a
Context Menu has been shown? I tested this with a MainMenu instead and it
Disposed just fine. Could this be a memory leak in the Framework? I'll be
happy to post a sample project demonstrating it, but in order to see the
problem you will require some sort of profiling tool capable of examining
the Heap. I'm using VS.NET 2003, Framework 1.1, WinXP Pro.

Many thanks,
Alex Clark
 
Alex, Here's our solution
-------------------------
Failing to dispose of MainMenu and ContextMenu objects in
a Form or UserControl can sometimes result in
the Form or Control not being garbage-collected when the
Form or UserControl are closed. To fix this, make
sure you explicitly call Dispose() on a MainMenu or
ContextMenu object within the Dispose(bool disposing)
method of the Form or UserControl. This problem generally
seems to happen only when the Enabled property of
a MenuItem contained by the MainMenu or ContextMenu is set
to false, but it shouldn't hurt to apply the fix
in all cases.

---------------------
 
Hi Bruce
Thanks for the suggestion but it didn't have any effect unfortunately, the
Form still won't dispose :-(

Cheers,
Alex Clark
 
Hi Alex,

From my simple test program, I didn't see the leak after explicitly
disposing the context menu.
Here is some code snippet
<code on main form>
private void button2_Click_1(object sender, System.EventArgs e)
{
Form f = new Form2();
f.ShowDialog();
f.Dispose();
f = null;
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
GC.WaitForPendingFinalizers();
}
</code on main form>
<code on second form>
protected override void Dispose( bool disposing )
{
if( disposing )
{

if(components != null)
{
components.Dispose();
}
contextMenu1.Dispose();
}
base.Dispose( disposing );
}
private void menuItem1_Click(object sender, System.EventArgs e)
{
MessageBox.Show("dsfsassdfasfsa");
}
</code on second form>
Have you called GC.Collect and GC.WaitForPendingFinalizers methods before
using CLR Profiler?
Try calling these two methods twice before profiling to ensure the object
has been finalized.
If the Form2 is still there, does the CLR Profiler show who holds the
reference of the form?
If you still have problem on this issue, please let me know more about it,
I'll follow up with you on this issue. Thanks!

Best regards,

Ying-Shen Yu [MSFT]
Microsoft Online Partner Support
Get Secure! - www.microsoft.com/security

This posting is provided "AS IS" with no warranties and confers no rights.
This mail should not be replied directly, "online" should be removed before
sending.
 
Hi Ying-Shen,

I've added a routine called RecursiveDispose(container as Control) which
basically goes through all controls on the form and the controls contained
within them, calling dispose on each one recursively. I'm calling this from
within the Dispose method of the form. By doing this, I can be assured that
absolutely EVERYTHING is getting Dispose called on it, including the
ContextMenu and all of it's menu-items.

Next, I've added about 5 images to the form to increase it's memory
footprint so that I can see in Task Manager if the memory useage is going up
or down.

Also, I've altered the code to show Form2 as follows:

Dim f As Form2 = New Form2
f.ShowDialog()
f.Dispose()
f = Nothing

GC.Collect()
GC.WaitForPendingFinalizers()
GC.Collect()
GC.WaitForPendingFinalizers()

Sadly, none of this has cured the problem. Please note that it is ONLY when
I show the context menu on Form2: If I don't show it, there isn't a memory
leak.

I load the application, check the mem useage, click the button to load
Form2, the memory has (of course) gone up. I then close Form2, the
mem-useage goes down to what it was originally (within a tolerance of a few
kb). All's well and good.

HOWEVER, if I repeat the process but this time click the button on Form2 to
show the Context Menu and then close it, the mem-useage DOES NOT DROP, it
stays pretty much identical to what it was while Form2 was open.

I've been using the .NET Memory Profiler by SciTech Software to confirm that
in these cases, Form2 is not being unloaded from memory. Is there any way I
can forcibly kill a Form from memory? Is there any other workaround? This
is pretty urgent as it's having a detrimental affect on a large project
which is due to go live this month, and my customer is getting more and more
disenchanted with .NET!

Cheers,
Alex Clark
 
Hi Alex,

Could you send me your repro sample?
I'll take a look at it and see if I can figure out something.

Thanks!

Best regards,

Ying-Shen Yu [MSFT]
Microsoft Online Partner Support
Get Secure! - www.microsoft.com/security

This posting is provided "AS IS" with no warranties and confers no rights.
This mail should not be replied directly, "online" should be removed before
sending.
 
Hi,

I did a quick check of this memory leak. Using our profiler it was easy to
see that the Form is being referenced by a MenuItem that has been added to
the static Hashtable "allCreatedMenuItems". By disassembling the MenuItem
class it's also quite easy to see what's causing the MenuItem to be added to
the hashtable. When the menuitem is shown AND when it's updated, a
MENUITEMINFO class is created. When creating the MENUITEMINFO a uniqueId for
the MenuItem is assigned (allowing the MenuItem to be identified when
receiving Win32 messages). The MenuItem is then added to the
"allCreatedMenuItems" hashtable. In the Dispose function the MenuItem is
removed from this hashtable. The problem is that the MenuItem is added to
the hashtable, with a new uniqueId, each time the MenuItem is updated.

The following code can be found in MenuItem.CreateMenuItemInfo:

<snip>
this.uniqueId = ++MenuItem.createdMenuItemsCounter;
MenuItem.allCreatedMenuItems.Add( this.uniqueId, this );
</snip>

If the above code was replaced with the following, there would be no memory
leak:

<snip>
if( uniqueId == -1 ) // uniqueId is assigned to -1 in the constructor.
{
this.uniqueId = ++MenuItem.createdMenuItemsCounter;
MenuItem.allCreatedMenuItems.Add( this.uniqueId, this );
}
</snip>

Another sideeffect of this bug is that the allCreatedMenuItems hashtable
grows each time a MenuItem is changed (e.g. changing the Enable property),
and never shrinks (except for one entry being removed in MenuItem.Dispose).

The problem with the growing hashtable has actually been reported as a bug
previously in this newsgroup.

The only workaround I can come to think of is to use reflection to explictly
remove all added entries from the hashtable.

Something like:

static Hashtable allCreatedMenuItems;
void DisposeMenu( Menu menu )
{
if( allCreatedMenuItems == null )
{
Type miType = typeof( MenuItem );
FieldInfo htField = miType.GetField( "allCreatedMenuItems",
BindingFlags.Static | BindingFlags.NonPublic );
allCreatedMenuItems = (Hashtable)htField.GetValue( null );
}

// Find out keys to remove.
ArrayList keysToRemove = new ArrayList();
foreach( DictionaryEntry de in allCreatedMenuItems )
{
if( de.Value == menu )
keysToRemove.Add( de.Key );
}
// And remove them.
foreach( object key in keysToRemove )
{
allCreatedMenuItems.Remove( key );
}

// Dispose children
foreach( MenuItem mi in menu.MenuItems )
{
DisposeMenu( mi );
}
}

Add the above code to the Dispose method of the Form containing the Menu
(the Menu and all its MenuItems should already have been Disposed, which is
done automatically by the code generated by the forms designer):

/// <summary>
/// Clean up any resources being used.
/// </summary>
protected override void Dispose( bool disposing )
{
if( disposing )
{
if (components != null)
{
components.Dispose();
}
DisposeMenu( contextMenu1 );
}
base.Dispose( disposing );
}

I hope this solves your problems and that this message answers your question
in the e-mail you sent to us.

Best regards,

Andreas Suurkuusk
SciTech Software AB
 
Hi Alex,

Thanks for your repro.

In debugging, I found the the method RecursiveDispose(Me) actually didn't
run,
so the context menu didn't be disposed.
You may change your Dispose Method and RecursiveDispose method as below, it
should release the form2.
<code>
'Form overrides dispose to clean up the component list.
Protected Overloads Overrides Sub Dispose(ByVal disposing As Boolean)
If disposing Then
If Not (components Is Nothing) Then
components.Dispose()
End If

ContextMenu1.Dispose()
RecursiveDispose(Me)

End If
MyBase.Dispose(disposing)
End Sub

Private Sub RecursiveDispose(ByVal container As Control)
For Each ctl As Control In container.Controls
'It might be better to use ctl.Dispose(), Let each control
decide how to dispose itself.
RecursiveDispose(ctl)
Next

' needn't call container.Dispose, since you are already in the
Dispose method of the container.
' Or we will have an dead loop ends up with StackOverFlow exception.
'container.Dispose()

End Sub
</code>

Does it solve your problem?
If you still have problem on it, please be free to reply this thread.
Thanks!

Best regards,

Ying-Shen Yu [MSFT]
Microsoft Online Partner Support
Get Secure! - www.microsoft.com/security

This posting is provided "AS IS" with no warranties and confers no rights.
This mail should not be replied directly, "online" should be removed before
sending.
 
Hi Andreas,

Thanks for the code. I'll convert it to VB and use it to ensure all
menu-items are properley killed at dispose-time in my app. I think the
solution Ying-Shen has given me will sort it, but knowing that there is an
actual problem in the framework (and where it is, how to override the
behaviour etc) is certainly helpful. Your profiler is an excellent tool by
the way :-)

Cheers,
Alex Clark
 
Back
Top