The notification mechanism on Scopes may leak.

[sialcasa]

Same issue as #378

Discussions there

[grill2010]

Hi,

we are currently tracking down a memory leak in our application and the the root points to DefaultNotificationCenter (currentNotificationCenter) and DefaultNotificationCenter$ChannelObserverMap(channelObserverMap) from the package de.saxsys.mvvmfx.utils.notifications. Is this bug already fixed as the related issue #378 is already closed.

However I have to admit that the memory leak occurred just recently and we did some refactoring in our code and before we didn't encounter this problem. So that could also be the problem, I just wanted to know if this issue is resolved or still open.

[manuel-mauky]

This issue here was a kind of reminder to take a closer look into the implementation. We created the issue to not forget it when we were discussing the whole scopes concepts. It was not the result of an actual bug but just a reminder task.

At the moment we have no actual evidence that there is any memory leak in the scopes implementation. We haven't found any bugs in our apps and haven't received bug reports in that area. Of cause this doesn't mean that there actually aren't any bugs.

The NotificationCenter is another story. In the past we had some issues with memory leaks with notifications and have fixed them as far as possible. Some problems can't be fixed and have to be taken care of while implementing your app. I've writen something on this topic in our wiki: https://github.com/sialcasa/mvvmFX/wiki/Notifications#notice-on-memory-leaks

The basic problem is that the DefaultNotificationCenter is a global map. If you register a listener then the listener keeps a reference to the surrounding class (a ViewModel for example) and the NotificationCenter keeps a reference to the listener so effectively the ViewModel can't be garbage-collected until you remove the listener. The listener can't be removed automatically so you have to do it by yourself. Or you can use the `WeakNotificationListener' as descriped in the wiki.

Actually, the global notification mechanism can be quite handy but it also can be tricky.

Have you fixed your problem in the meantime or do you need some help? Maybe I can assist you if you share some more details.

[grill2010]

Okay I see, thank you for the information. Yes we fixed our issues and so far everything looks stable. If we encounter again some problems I will let you know but as I thought the problem was not related to mvvmFX.

[grill2010]

Edit: we maybe found something as it seems there is a possible issue in the channelObserverMap. As far as I have seen the channelObserverMap uses the hash from the object as key and and as value it uses a ObserverMap. The value ObserverMap gets removed as soon as it doesn't have any more entries left upon unsubscribe. However channelObserverMap never gets deleted when is has no more entries left, as we have a very dynamic application with many reloading objects this could be a problem at some point. (We generate and destroy many views and viewmodels during the runtime and our application should run 24/7). Could this be a memory leak or does it get somehow deleted anywhere else?

[manuel-mauky]

Ok, you got a point. The channelObserverMap holds an observer-map for each channel.
When you use the notifications for a specific viewModel then the viewModel itself is the channel. If you dynamically create new ViewModels then new channels are added and old channels and their observer-maps won't be cleaned up.

When we created the "channel" feature we had in mind a limited number of channels - for example one channel per feature. But of cause your usage of dynamically creating new viewModels (and therefore new channels) is also perfectly valid.

I think the severity is limited here because it doesn't keep references to the viewmodels itself (this is the reason why we use the hashes and not the objects itself) and so doesn't prevent the viewModels from being garbage-collected. However, the size of the channelObserverMap will grow over time and be filled with empty HashMaps, which of cause is also a problem.

To fix this I will add code to check if an ObserverMap is empty after the observers were removed and if so delete the observermap from the channelObserverMap.

[grill2010]

Cool, many thanks for your help and your quick fix. :)