Memory leaks when using WPF Diagrams with Prism

Hello!

We are using WPF Diagrams component with MVVM and Prism. The application has some modules that can display a diagram. We have a lot of elements on the diagram (more than thousand) and their style can be different in different modules.

In each module we have a view with the following code:

<ms:DiagramSurface Name="ds"
                       Grid.Row="0"
                       BorderThickness="1,0,1,1"
                       Diagram="{Binding DG, Mode=TwoWay}"
                       Formatter="{StaticResource Formatter}"
                       HorizontalSnapSpecifier="{ms:SnapToInterval 10}"
                       SelectedElementsChanged="DsSelectedElementsChanged"
                       SelectionMode="Extended"
                       ShowConnectionPoints="True"
                       SnapLineDrawing="{StaticResource SnapLineDrawing}"
                       VerticalSnapSpecifier="{ms:SnapToInterval 10}"
                       ZoomMode="ControlMouseWheel" />

And also we have ViewModels with public property DG:

public Diagram DG
    {
        get { return _dg; }
        set
        {
            _dg = value;
            this.RaisePropertyChangedEvent("DG");
        }
    }

In constructors we get Diagram by dependency injection like this:

public Module1DiagramViewModel(IUnityContainer container, EventAggregator eventAggregator, Diagram diagram)
    {
        _container = container;
        _eventAggregator = eventAggregator;

        DG = diagram;

        this.Initialize();
    }

So, in each module we have the same elements on the diagram, but may be in different shapes.

But when we switch between modules we can see that the usage of memory increases. Each switching adds about 150 - 200mb (depends on module). So several switching lead to out of memory exception. We tried to understand what is the reason, and execute our application under profiler. We get the picture that I have attached. As we see, byte arrays and WeakReference take the most part of total used memory. But we don't use byte arrays and WeakReference in our code.

Could you please help us, and say what can be the reason of such behavior?

//Dmitry

Hello Dmitry

To clarify: You have some modules each which display the same Diagram instance in different ways. Each module has its own DiagramSurface instance in the view.

You mention that you switch between modules. Are you switching between the same two modules to cause the memory to increase? or are you switching between many different modules? If you switch back to a module that you've viewed before, does the memory continue to increase? Each time you switch modules, are you creating a new instance of the module? or are you obtaining the module from some kind of module store?

My initial guess is that each time you switch modules, a new instance of the DiagramSurface is being created. When the previous DiagramSurface is removed from the visual tree it probably still has a reference to the Diagram model meaning it can not be consumed by the garbage collector. This would mean all the visual elements and event handlers start filling up the memory. When you switch modules, try setting the Diagram property of the previous DiagramSurface to null. You could probably do this by setting the DG property of the previous modules' ViewModel to null. Try this out and let me know if the situation is improved, resolved, or unchanged.

Jason Fauchelle

Hello Jason!

Yes, you are right. Each module displays the same diagram in different ways. Each module has its own DiagramSurface instance (some modules can contain more than one instance of DiagramSurface, for example as mini map).

I'm switching between modules that contain DiagramSurface instance to cause the memory to increase (I have only three modules in the application and they all have at least one DiagramSurface instance). If I switch back to a module that I've viewed before the memory continues to increase. Each time when I switch modules, a new instance of the view with DiagramSurface is created. But the model is created once (we use MVVM pattern, may be you know it).

I tried to set Diagram property of the previous DiagramSurface to null, but it means I set the Diagram instance that I used in that module to null. I get this Diagram only when instance of the model is created, so next time when I switch back to this module I get this Diagram empty. Are any other possibilities to clean the DiagramSurface?

//Dmitry

Hello Dmitry

There are no other ways to clear the DiagramSurface, but if there was, the effects would be the same as setting the Diagram property to null. (Visual need to be destroyed and event handlers through the model need to be removed, so the Diagram property may as well be null).

When you set the Diagram property of the previous DiagramSurface to null, was the memory issue improved? (You should use a manual call to the garbage collector whenever you do this to make it easier to measure this). If there is an improvement to the memory issue, then I suggest exploring this idea some more. Would it be possible for you to add some methods to the modules to Dispose and Reload the module? When the modules are switched, you dispose the current one, and reload the new one. Alternatively, rather than switching to the same instance of a previous module, is there a way to always create a new instance of the module so that it gets loaded with the Diagram?

This issue is a bit difficult to resolve without seeing the code, so this explanation may be a bit vague. Try something along this approach and let me know how it goes.

Jason Fauchelle

Hello Jason!

I can't see any improvements with the memory issue when I set the Diagram property to null and call for the GC then.

Is it possible for you to perform the following test - create a huge diagram, wait while it is displayed, evaluate the memory usage by the application, then set the Diagram property to null, call for the GC and after that evaluate the memory again. Could you see the decreasing of memory usage in that case or not? May be I'm doing something wrong...

//Dmitry

Hello again

We just implemented some "hacks" now and see that setting the Diagram property to null can solve the problem. Memory increases not so fast.

But also we see that setting Diagram to null takes much more time than we expected. Could you clarify why it can be so? Is it normal?

//Dmitry

Hello Dmitry

Good to hear you can see an improvement to the memory issue when setting the Diagram to null. I've tried out the test you described. After calling the GC, the memory drops a little bit, but not back down to what it was before creating the diagram. After leaving the application running for a while, the memory continues to drop back down to where it was. So the memory is certainly released.

In the next nightly build, the performance for setting the Diagram to null will be greatly improved.

Jason Fauchelle

Hello Jason

I can see that setting the Diagram property to null works much faster now. Thank you!

//Dmitry

Hello Jason,

we have also some problems with memory leaks in our application. I provide a sample application that I used to find out what caused these problems. My preferred tool is ANTS memory profiler. The steps to reproduce these leaks are:

1. Start the application and memory profiler
2. Add two nodes and a connection between them
3. Take a memory snapshot
4. Press the "New" menu button
5. Take a second memory snap shot
6. Filter the class list to show only the surviving objects
7. Inspect the “Instance Retention Graph”

The first problem is that DiagramConnectionElement adds an event handler to ConnectionIntersectionCache in the ctor which will never be released: ConnectionIntersectionCache.IntersectionsChanged += new EventHandler(ConnectionIntersectionCache_IntersectionsChanged); (see IntersectionCacheLeak.png)

The second problem is that FloatingConnectionpoint has a static field _instance that doesn’t get cleared. (see FloatingConnectionPointLeak.png)

The third problem is that AStarPathfinder holds a static field _runners that doesn’t get cleared. (see AStarLeak.png)

As a result the whole object tree is kept in memory including all our ViewModels and BusinnessObjects. As a result we get many side effects on instances that reside in memory and try to respond to events from the diagram that is no longer visible. Also the memory consumption increase with each new diagram opened by the user.

Thanks for the repro project.

It will be at least a week before we can look into this. I'll get back to you when I'm working on this.

Jason Fauchelle

Hello We have started looking into this. Not sure how long this will take but I'll keep you updated.

Jason Fauchelle

Hi Jason,

thanks for keeping me up to date. Today I did an experimental build of the diagram component, to see what I would do to get rid of the memory leaks. Finally I came up with the code below ! I left out any interfaces and presented only the relavant code changes in each class. Hope I could give you some ideas

Markus

DiagramSurface.cs

public DiagramSurface()
{
    ...

  Unloaded += (sender, args) =>
   {
     smartScrollTimer.Stop();
     smartScrollTimer.Tick -= SmartScrollTimer_Tick;
   };
}

private void OnDiagramChanged(DependencyPropertyChangedEventArgs e)
{
  IDiagramModel oldDiagram = e.OldValue as IDiagramModel;
  if (oldDiagram != null)
  {
    DetachCollectionChangedHandlers(oldDiagram);
    DetachDiagramModelEventHandlers(oldDiagram);
    ClearAllChaches();
  }
    ....
}

private void ClearAllChaches()
{
  AStarPathfinder.Clear();
  ConnectionIntersectionCache.Clear();
  _diagramElements.Clear();
  _selectedItems.Clear();
  SetValue(SelectedItemsPropertyKey, new ReadOnlyCollection<object>(_selectedItems));
  this._diagramGrid.Clear();
  this._recycledNodes.Clear();
  this._recycledConnections.Clear();
}

AStarPathfinder.cs

internal static void Clear()
{
  _runners.Clear();
}

ConnectionIntersectionCache.cs

internal static void Clear()
{
  _intersections.Clear();
  if (IntersectionsChanged != null)
  {
    foreach (EventHandler d in IntersectionsChanged.GetInvocationList())
    {
      IntersectionsChanged -= d;
    }
  }
}

DiagramGrid.cs

internal void Clear()
{
  foreach (var diagramGridBucket in _buckets)
  {
    foreach (var node in diagramGridBucket.Value.Nodes)
    {
      node.Clear();
    }
  }

  _boundsCache.Clear();
  _connectionBoundsCache.Clear();
  _buckets.Clear();
  _nodeBuckets.Clear();
  _segmentBuckets.Clear();
}

DiagramNodeBase.cs

public virtual void Clear()
{
  if (RotationChanged != null)
  {
    foreach (EventHandler d in RotationChanged.GetInvocationList())
    {
      RotationChanged -= d;
    }
  }

  if (BoundsChanged != null)
  {
    foreach (EventHandler d in BoundsChanged.GetInvocationList())
    {
      BoundsChanged -= d;
    }
  }

  if (this.ConnectionPoints != null)
  {
    foreach (var pt in this.ConnectionPoints)
    {
      pt.Clear();
    }
  }

  ConnectionPoints.Clear();
}

DiagramConnectionBase.cs

public virtual void Clear()
{
  foreach (var pt in ConnectionPoints)
  {
    pt.Clear();
  }
  ConnectionPoints.Clear();
}

DiagramConnectionPointBase.cs

public virtual void Clear()
{
  if (PositionChanged != null)
  {
    foreach (EventHandler d in PositionChanged.GetInvocationList())
    {
      PositionChanged -= d;
    }
  }
}

FINALLY IN THE SAMPLE CODE

LogicNodeViewModel.cs

delete the "Connections" property

Hello Markus

Thanks for your patience in this and the code samples.

I've resolved all the memory leaks I could reproduce using your repro app, and also included some of the code you suggested. The main thing I couldn't reproduce was the FloatingConnectionPoint leak. All the fixes will be available in the next nightly build. Let me know if there are any leaks I missed.

Jason Fauchelle

Hi Jason,

I can see a hughe progress. The FloatingConnectionPoint leak is still there. Maybe you have to move the nodes and the connection a liitle bit before taking the second snapshot. The reason is the static "Instance" field on the FloatingConnectionPoint class. This class owns an event handler named "PositionChanged" and thefore the classes that are attached to this event are kept in memory.

Sorry to bother you again with this problem.

Markus

DiagramSurface.cs:

OnDiagramChanged ... FloatingConnectionPoint.Instance.Clear();

FloatingConnectionPoint.cs:

public void Clear()
{
  if (PositionChanged != null)
  {
    foreach (EventHandler d in PositionChanged.GetInvocationList())
    {
      PositionChanged -= d;
    }
  }

No worries Markus

I was able to reproduce this remaining memory leak and so this will be resolved in the next nightly build.

Jason Fauchelle

Hi Jason,

I found a new memory leak that causes trouble in our application. The internal static class ConnectionIntersectionCache holds a list of _intersections that never gets cleared. I have attached an image from the ANTS profiler. I suggest to implement a clear method on the ConnectionIntersectionCache and call that in the DiagramGrid clear method.

Thanks Markus

Hello Markus

Thanks for pointing this out. The next nightly build will contain a fix similar to what you described. Rather than clearing all connections though, the segments of each connection will be iterated and removed from ConnectionIntersectionCache. This is so that intersections for other diagrams are not affected. Ideally we will improve this solution in the future so that the intersections are managed by the diagram models, rather than statically. Let me know if this fully solves the problem at your end.

-Jason Fauchelle

Hello Jason, the latest nightly build removed the leak. I want to say goodbye because my team left the project and the product will be released soon. Because of my good experience with wpf diagram and the awesome support, I will recommend the mindscape controls in my next projects too.

Thanks Markus

Hello Markus

Great to hear the leak has been resolved. Thanks for choosing Mindscape WPF Diagrams and your kind words. All the best for your product release!

-Jason Fauchelle