Memory leaks

[jack9267]

I have Visual Studio's leak detection enabled and it's going crazy when I use RmlUi.
Source\Core\ObserverPtr.cpp has a GetPool function that does a dirty quick "new".

static Pool< ObserverPtrBlock >& GetPool()
{
	// Wrap pool in a function to ensure it is initialized before use.
	// This pool must outlive all other global variables that derive from EnableObserverPtr. This even includes
	// user variables which we have no control over. For this reason, we intentionally let this leak.
	static Pool< ObserverPtrBlock >* pool =  new Pool< ObserverPtrBlock >(128, true);
	return *pool;
}

Can we please just fix this intentional leak? It makes it hard for me to debug my own leaks.

[mikke89]

Hm, I never had a problem here with valgrind.

There isn't really anything to "fix" here, because this is intentional and necessary. I think the comment explains the situation pretty well. Is there no way you can add this to an ignore list or similar?

[jack9267]

You can't add things to the ignore list, users shouldn't be keeping globals from the lib, there should be Rml::ShutDown() freeing the pool. The user should avoid calling ShutDown if they keep these globals. I checked and pool usage is 0 by the time I call Rml::ShutDown so it seems weird it's intentional.

[jack9267]

The list goes on and on.

[mikke89]

Hm, the issue is that anything that eg. derives from Rml::EventListener then needs to outlive Rml::Shutdown, which will lead to unexpected crashes for many users, and more headaches than it solves.

In my opinion, it is perfectly reasonable for some (one-time) heap allocations to outlive the application, this to me sounds more like a limitation of the tool.

[jack9267]

The user should be deleting the Rml::EventListeners before calling Rml::ShutDown. Perhaps an optional bool to ShutDown or an additional function to shutdown the pool? I contain everything within a class so I don't keep anything from the library, to me it sounds like a user problem if they keep static globals. Alongs the ability to free the pool exists I'm happy. I need to be able to free up all the memory associated with the library for my needs. This is all for a multiplayer mod for the GTA series.

I will end up needing to free the module and load it again so this will cause issues without this functionality.

A second alternative is Rml::ShutDown checking if the pool is empty and simply freeing it which covers people having globals vs people who don't.

[jack9267]

Let me know though, I can make a pull request to even just add a function to free the observer pool.

[mikke89]

Is that the only one place causing issues?

Yeah, I'd be fine with a function to release the pool. Maybe next to the release textures and geometry functions? A PR would be welcome :)

[jack9267]

That sounds about right actually, now you mention the release textures and geometry functions. I'd still recommend freeing the pool if its empty in Rml::ShutDown (for those who don't hold the weird static references). Any naming preferences here for functions or the variable for the pool global?

Another alternative is to simply make the pool not a pointer (static) but I do prefer the function since I can call that when I need the memory freed.

[mikke89]

Great. Yes, actually, I agree that cleaning it up automatically if it's empty in Rml::Shutdown would be good. If we want a function in addition, maybe ReleaseMemoryPool(s)? But I'm open for suggestions.

[jack9267]

Just quickly wrote up a quick PR (not tested yet!)

#265

[mikke89]

Fixed in #265, thanks!