dotnet / dotnet/wpf

Memory Leak when Effect assigned to UIElement not been frozen

Open
#6,782 4 comments 4 reactions 0 assignees View on GitHub
Investigate Performance
Dominant language
C#
Stars
7.7k
Forks
1.3k
Avg merge
1d 11h
Merged PRs (30d)
61

Description

* .NET Core Version: 6.0.203
* Windows version: 21H2
* Does the bug reproduce also in WPF for .NET Framework 4.8?: I don't know


**Problem description:**
I define DropShadowEffect in global Application.Resources> in App.xaml, and I use it for a Border.Effect property.
The Border is in some Window. All windows I created and closed stay in memory.

I found that the Window object is is held by the Border, and the Border is held by an EventHandler, and the EventHandler is held by DropShadowEffect. EventHandler has reference to EffectChanged method.

I think I have found the problem.
When an `UIElement.Effect` property is assigned, the setter (`Visual.VisualEffectInternal.set`) adds strong event handler for `EffectChanged` method

https://referencesource.microsoft.com/#PresentationCore/Core/CSharp/System/Windows/Media/Visual.cs,2976
From ILSpy
```c#
// System.Windows.Media.Visual
using System.Windows.Media.Effects;

internal Effect VisualEffectInternal
{
get
{
if (NodeHasLegacyBitmapEffect)
{
return null;
}
return EffectField.GetValue(this);
}
set
{
Effect value2 = EffectField.GetValue(this);
if (value2 == value)
{
return;
}
if (value != null && !value.IsFrozen)
{
value.Changed += EffectChangedHandler; // <-----
}
if (value2 != null)
{
if (!value2.IsFrozen)
{
value2.Changed -= EffectChangedHandler; // <-----
}
DisconnectAttachedResource(VisualProxyFlags.IsEffectDirty, value2);
}
SetFlags(value != null, VisualFlags.NodeHasEffect);
EffectField.SetValue(this, value);
SetFlagsOnAllChannels(value: true, VisualProxyFlags.IsEffectDirty);
EffectChanged(null, null);
}
}
```

Event handler is removed only if an Effect property is changed. _(Am I wrong?)_

Analyzing this code, you can see that the event handler has not been added when the Effect object is frozen (IsFrozen=true).

I found a page, on which was written how to switch IsFrozen in XAML.

https://docs.microsoft.com/en-us/dotnet/desktop/wpf/advanced/freezable-objects-overview?redirectedfrom=MSDN&view=netframeworkdesktop-4.8#freezing-from-markup
```
xmlns:PresentationOptions="http://schemas.microsoft.com/winfx/2006/xaml/presentation/options"
PresentationOptions:Freeze="True"
```
But, it's not working.

One solution to the memory leak I have found is to define each DropShadowEffect localy in a Border.


**Actual behavior:**
Effect setter (System.Windows.Media.Visual.VisualEffectInternal.set) use strong reference for event handler.

**Expected behavior:**
Effect setter (System.Windows.Media.Visual.VisualEffectInternal.set) should use weak reference for event handler (e.g. WeakEventManager)

**Minimal repro:**
https://github.com/marbel82/UIElementEffectMemoryLeakRepro

Click [Create LeakWindow], close the Window, click GC.Collect, Pause Visual Studio, Memory Usage/Take Snapshot and find LeakWindow

![image](https://user-images.githubusercontent.com/31281571/178527091-0f8b1fe6-7ce5-4b5f-b569-b356f19ae6d9.png)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.