DS4Windows icon indicating copy to clipboard operation
DS4Windows copied to clipboard

Memory leak when controllers are disconnected

Open Kanuan opened this issue 2 years ago • 11 comments
trafficstars

There is some memory leak related to resources not being freed when a controller is disconnected.

My log: DS4W v3.2.14 Log, memory leak on controller disconnect.txt

Memory usage when first starting (controller connected already): image

Memory usage after reconnecting a controller 15 times: image

Kanuan avatar Sep 06 '23 23:09 Kanuan

The memory usage probably probably does not goes down on its own after some time. I noticed this issue after a whole day of making tests with my controller that required constant reconnection and then opened task manager and seeing DS4Windows at almost 300MB of RAM usage

Kanuan avatar Sep 06 '23 23:09 Kanuan

I'll have to look into it. I would hope there is no outdated reference being held somewhere but there must be. Even with server mode garbage collection enabled, the memory being used for old DS4Device instances should be cleared well before that point.

Ryochan7 avatar Sep 10 '23 19:09 Ryochan7

Looked into this a little bit. DS4Device instances and events seem to be freed as intended when a controller is unplugged. The garbage collection method (GC.Collect) is being called every 30 seconds but at least the current .NET Runtime does not seem to actually run garbage collection every time when requested; it used to be more active and would perform garbage collection every time. Seeing a lot of references for WPF and WPF Extended Toolkit related objects while running the performance profiler.

DS4Windows using server mode garbage collection does make the program use much larger portions of usable memory. Based on what I have seen, memory usage does not seem to be unbound and .NET will eventually free memory as it is needed by the system (while launching a game for example).

Not sure it really matters but I might try something similar in DS4MapperTest. Several aspects of that program are different and the code is better structured IMO.

Ryochan7 avatar Sep 14 '23 05:09 Ryochan7

Funny enough, it looks like DS4MapperTest does not suffer from this problem. Not sure why. That project uses a different GUI toolkit (HandyControl) but most of the other software stack is similar to DS4Windows. Over the past year, many changes have been carried over from DS4MapperTest to DS4Windows.

Ryochan7 avatar Sep 15 '23 05:09 Ryochan7

Just happened to notice something while experimenting. Looks like the version of Nefarius.Utilities.DeviceManagement that DS4Windows uses has a memory leak problem when it comes to reading device properties. Marshal.AllocHGlobal is called to make the buffer to store the byte array for a property. If the method actually succeeds then the memory is not freed so that chunk of memory is lost; Marshal.FreeHGlobal is never called. Several memory leaks will occur for each virtual device check performed.

An example showing why having many return calls in a method is usually a bad idea. Going to have to check upstream and see if the problem still exists out of curiosity.

Ryochan7 avatar Sep 17 '23 18:09 Ryochan7

My C++ mindset. Looks like .NET plays nicely with this scenario and will run the finally block regardless. No memory leak from that method after all. Still bad form IMO.

https://github.com/nefarius/Nefarius.Utilities.DeviceManagement/blob/master/src/PnP/PnPDevice.Properties.cs#L55

Ryochan7 avatar Sep 17 '23 18:09 Ryochan7

Been following the thread. So you think memory will be freed if the general RAM usage starts reaching the limit, right? Will try testing this but seems odd regardless.

Kanuan avatar Sep 26 '23 11:09 Kanuan

Installed DS4Windows under an hour ago and immediately noticed that it uses a lot of memory. It's currently at 878 MB. Restarting the program puts it back to 200 MB (also seems high). Opening a profile and hitting Save jumps usage up to 300 MB. Repeating the process keeps growing the usage.

DualSense controller DS4Windows v3.2.17 Windows 11

redactedscribe avatar Oct 28 '23 11:10 redactedscribe

Regarding comment from @redactedscribe ...

I looked into the ProfileEditor memory leak issue with dotMemory.

Testing was done with ServerGarbageCollection set to false in project file. I tried opening and then immediately closing the ProfileEditor control 10 times (by clicking Cancel). This resulted in a memory usage increase from 80MB to over 170MB (version 3.2.20, when viewing in TaskManager). This is because all 10 instances of ProfileEditor remain alive in memory - viewing retention paths shows that the main reason is unregistered event handlers.

retention

#3176 fixes the issue by adding the UnregisterEvents method to ProfileEditor. With this change dotMemory shows no live instances of ProfileEditor in memory after opening and closing the ProfileEditor control 10 times. This resulted in a memory usage increase from 80MB to over around 130MB (around 40MB less).

When ServerGarbageCollection is set to true (which is the default in DS4WinWPF.csproj) memory usage in both cases rises to above 500MB. I guess this is because ServerGarbageCollection works differently. When attaching dotMemory and forcing garbage collection, it can be seen that the memory usage drops significantly in the version which contains the fix.

kurtanr avatar Nov 26 '23 20:11 kurtanr

Even after the 3.2.21 update, which I assumed addressed the source of my problem, I am still encountering issues I assume to be memory leak related that makes long-term use of this application untenable. When leaving the program running in the background, with PS4 controller plugged in and not in use, eventually the entire PC will become choppy and stutter, affecting audio, video, input, everything. The only solution is to restart the computer and not run DS4Windows.

redmagejoe avatar Dec 05 '23 20:12 redmagejoe

Profiles editing memory leak issue as described in my previous comment still present in 3.3.0. If #3176 was supposed to fix this, it hasn't on my end at least.

redactedscribe avatar Dec 16 '23 21:12 redactedscribe