Prism: Memory leak in RegionViewRegistry

[brianlagunas]

How about a PR to fix it?

[dvorn]

@brianlagunas I do not think there is a problem with RegisterViewWithRegion. It is a misuse of it.

@Sid-R Removing a view from a region does not assume removing the registration of the view with the region. The idea of RegisterViewWithRegion is to populate a region before the region is even created. The region may be created later and it will be auto-populated. The region may then be deleted and created again, and it will be populated again. The registration will live for the lifetime of the application. Usually RegisterViewWithRegion is called only once, when a module is loaded, so there is no problem.

Seemingly what you are doing is attempting to navigate to your view by calling RegisterViewWithRegion multiple times, not only in module initialization. This is wrong, you should use the Navigate method instead.

[brianlagunas]

@dvorn great analysis! I completely agree with your assessment. I couldn't have said it better myself. As you can see my answer was a result of me not having time to actually look into it :)

[Sid-R]

Thank you all for taking a look into this issue.

Interesting to note that
The registration will live for the lifetime of the application.

Actually RegisterViewWithRegion is not being called multiple times.
In our project we have 'heavy' modules that are being created and disposed dynamically throughout the lifetime. As intended RegisterViewWithRegion is being called only once during the Module.Initialize().

The problem is when the module is no longer needed we call Regions.Remove(...) and dispose the module. Our assumptions was that doing this will remove all references of this module from regionmanager and hence it will be collected by GC. But since as you mentioned this registration (string, List< delegate >) is for lifetime the disposed modules will survive the garbage collection as the delegate holds the module reference.

We realized the current RegionManager implementation is not meant for completely unregistering modules. Which in turn makes modules (which uses regionmanager) undisposable.

For now we have created a Helper class that provides a delegate to RegisterViewWithRegion instead of using lambda expression. It results in reference to helper class surviving inside regionviewmanager's ListDictionary but at-least our hack allows us to dereference and dispose our module(s) successfully.

Thanks again for your time and comments

[dvorn]

@Sid-R It is not quite clear what you mean by "dispose the module". The module is located in an assembly and once the assembly is loaded, it will remain in the current domain forever, with all its types. Normally the modules perform some initialization, in particular register some types in the container. The container, in turn, will be alive for the lifetime of the application. In your code there is something _container.Dispose();. If that is the main Prism container, all the Prism functionality (based on registration in the container) will be completely lost.

Seemingly you are using Prism in completely unorthodox way.

[dvorn]

@Sid-R And again, why don't you use RequestNavigate to populate your regions with views instead of RegisterViewWithRegion? All you need to do is register your view types (DefaultTaskView) in the container for navigation and then call RequestNavigate. The instance of the view will be created on demand when it is needed. The view type will always be in memory, but this is unavoidable as explained above.

[dvorn]

@Sid-R And if you have a very complicated process of initialization your module and creating the view which cannot be reduced to resolving the view via container, you can just add the view to the region without registration or navigation, e.g.

_regionManager.Regions[RegionNames.DefaultRegion].Add(view);

[Sid-R]

@dvorn Thanks a lot for "_regionManager.Regions[RegionNames.DefaultRegion].Add(view)".

In retrospect I concede that we are using prism in an unorthodox way in the sense that some of our modules are not loaded via catalog at startup as they are dynamically created at certain workflow events. (for such exceptional modules we manually create them and call IModule.Load()). So these modules when no longer needed are disposed as well manually by calling Dispose(). This is being done only for certain modules which are created and disposed manually inside some controller (thus unorthodox use as in prism, IModule are supposed to be created once and remain for lifetime of app).

I believe the right options for us would be-

• Do not register view for such temporary modules but add the view to the region directly via Regions.Add (thanks again for that!)

As for

_container.Dispose();

the container is childcontainer with scope limited to module being disposed so no impact on rest of prism application.

[dvorn]

@Sid-R Glad to hear you figured it out. Prism is a great and flexible framework.

I think everything is OK with your modules. They are just modules in a common sense of the word, as some pieces of code. But they are not Prism's modules: they do not implement IModule (look at your sample code above) and you do not use Prism module-related infrastructure to work with them. But there's nothing wrong with that.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.