search

benruehl / adonis-ui

Lightweight UI toolkit for WPF applications offering classic but enhanced windows visuals
https://benruehl.github.io/adonis-ui/
MIT License
1.74k stars 146 forks source link

Memory leak ? #109

Closed maximilien-noal closed 3 years ago

maximilien-noal commented 4 years ago

Describe the bug

A memory leak seems to live in adonisUI (tested with v1.16.0 but it seems to be older than that).

To Reproduce Steps to reproduce the behavior:

  1. Clone and startup RoleDDNG
  2. Make the Memory Usage tab from the Diagnostic Tools window visible
  3. Fire up the Dice Roller 'Window' (or "Lanceur de dés") with the keybinding (this is important) Ctrl-D and hold it.
  4. See the memory usage grow and grow over time. Moving the mouse along at the same time seems to make it grow faster.

Expected behavior

Same as the "memleak" branch on the same repo (where adonisUI was removed) : no memory leak.

Screenshots

master branch : memleak

This goes on forever and grows up to hundred of megabytes, even gigabytes, pretty quickly.

memleak branch : nomemleak

This quickly stabilizes around 175 MB.

Additional context

Granted, constantly firing a keybinding isn't an expected "real user" behavior, but still. No mem leak seems to happen if you close the 'window' first (by clicking on the 'x' button). Key code points :

benruehl commented 4 years ago

Thank you for your report and sorry for the late reply! I am really busy lately.

I cloned your repo but cannot run it unfortunately. It fails at resolving some Hammer.MDIContainer.Control dependency. Are there any setup instructions?

maximilien-noal commented 4 years ago

It's inside the Controls directory and should be a local reference. It's a fork of the nuget package of the same name. I think you could use the nuget version instead and still reproduce the issue. If it fails, I'll try to setup a minimal reproduction repo.

benruehl commented 4 years ago

I can't get it to run, sorry.

Switching to the NuGet version gives me an error that it does not support build target netcoreapp3.1.

Seems like Hammer.MDIContainer.Control itself is fine but it has a reference to Xceed.Wpf.Toolkit which cannot be resolved again. I noticed that this is placed as a local copy in your repo as well but does not get downloaded when cloning the repo. So I downloaded it manually and placed it in the empty directory wpftoolkit. Now the solution loads correctly but Visual Studio does not build anymore. It does not even start a build. It just pops up a message box that sais something like "Error: Interface is not supported".

The log file states: "Visual Studio ran into an unexpected problem with 'Xceed.Wpf.Toolkit'. System.ArgumentException: The project configuration "Debug|AnyCPU" was not found in the project manifest."

maximilien-noal commented 4 years ago

Sorry about this mess.

I removed wpftoolkit from all branches. It wasn't used anyway. it should work now.

benruehl commented 4 years ago

It runs now and I can reproduce the increasing memory consumption. I will try to find the source of this.

Mgamerz commented 4 years ago

Is this related to memory leak I reported here: https://github.com/benruehl/adonis-ui/issues/43 ?

In my fork of Adonis UI that I use for my application, I fixed this memory leak by changing event handlers for ripple host to weak event handlers. Ripple host seemed to leak memory through its event handlers not being unset when the control was destroyed, however there is nowhere I could find the hook up an unsetter that would reliably work, so I just used weak event handlers instead. I did not post it here since it probably wasn't really in the code style of this repo.

maximilien-noal commented 4 years ago

@benruehl : Thank you for looking into it.

My bet was on events. I do use some in my application, but they don't seem to be the source of the problem.

maximilien-noal commented 4 years ago

Is there anything we can do to help ?

benruehl commented 4 years ago

So, I finally found some time.

Last time I tried some VS memory profiling but didn't find anything unexpected. I am not used to debugging memory leaks so I might have made mistakes here.

Now I did a fresh restart. I cloned your repo again and noticed that the leak does not occur anymore. Looks like you fixed it by not instantiating new windows when holding down the keyboard shortcut. Anyway, I removed the check for existing windows and got the memory leak again. Then I disabled the ripple effect because @Mgamerz suggested it might be the reason. But nothing changed. I disabled the cursor spotlight as well but nothing changed again. I excluded all default styles and then excluded Adonis UI completely but nothing changed. And by "excluded Adonis UI" I mean I removed every single usage in all projects and the project references so that no DLL of Adonis UI can be found in the output directory.

There is nothing left of Adonis UI but I still get the memory leak.

I am a little confused now. To me it looks like this does not have anything to do with Adonis UI. Maybe it's an issue of the MDIContainers you used. Am I missing something here? Please tell me if I did a mistake and came to the wrong conclusion.

Mgamerz commented 4 years ago

Since you have an open source project, you should be able to get a free resharper license from jetbrains, if you're interested. I use their dotmemory program which I find much easier than visual studio's memory tools.

I trace memory leaks by attaching a weak reference in the constructor of my windows and objects and then monitor for when they go dead. Once I find something is not being released that should be, I use dotmemory to find the root path which is almost always some event handler not being unset.

I'm not sure the issue is the same for my app as the one here but that's how I traced it to Adonis. I used an older version when I reported my issue, it could be it was unintentionally fixed since. In my implementation I use weak event handlers which don't count as a reference for an object, but at a cost to performance. How much, I don't know. Whenever I get back to working on my app I might be able to take a look when I pull the latest code.

On Sun, Aug 23, 2020, 4:23 PM Benjamin Rühl notifications@github.com wrote:

Small update:

Last time I tried some VS memory profiling but didn't find anything unexprected. I am not used to debugging memory leaks so I might have made mistakes here.

Now I did a fresh restart. I cloned your repo again and noticed that the leak does not occur anymore. Looks like you fixed it by not instantiating new windows when holding down the keyboard shortcut. Anyway, I removed the check for existing windows and got the memory leak again. Then I disabled the ripple effect because @Mgamerz https://github.com/Mgamerz suggested it might be the reason. But nothing changed. I disabled the cursor spotlight as well but nothing changed still. I excluded all default styles and then excluded Adonis UI completely but nothing changed. And by "excluded Adonis UI" I mean I removed every single usage in all projects and the project references so that no DLL of Adonis UI can be found in the output directory.

I am a little confused now. To me it looks like this does not have anything to do with Adonis UI. Maybe it's an issue of the MDIContainers you used. Am I missing something here? Please tell me if I did a mistake and came to the wrong conclusion.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/benruehl/adonis-ui/issues/109#issuecomment-678832399, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAU4VFHFVGYQG33WIQ35LD3SCGJH5ANCNFSM4OQR74XQ .

benruehl commented 3 years ago

Closing this because it seems like the issue is not caused by Adonis UI and the ticket didn't receive any attention for quite some time now.