Memory allocation/dealloc problem in imgui_demo.cpp

[septag]

I think I've submitted this before. Although it's a minor thing, but it raises problems when using custom allcoators and ShowTestWindow() function.

There is a static ImVector decleration in imgui_demo.cpp@716 That causes the ImVector destructor to crash on program exit.
Is there any workarounds for this, instead of using static declerations ?

thanks

[ocornut]

There was one fix applied on Nov 5 but you have never answered the request for details in the old thread:
#303

What exactly is the issue? I would appreciate a more detailed explanation. Can't you have construct/destruct allocators matching? Is fixing imgui_demo for the case of using a custom allocator actually useful?

The one in line 716 could be replaced with a normal array + bool since the size is static. There's however at least another one in the custom drawing demo.

EDIT Also see #396

[septag]

There was one fix applied on Nov 5 but you have never answered the request for details in the old thread:

yeah, sorry for that, I got the bug and reported it but never had the time to get to it later on

What exactly is the issue? I would appreciate a more detailed explanation. Can't you have construct/destruct allocators matching? Is fixing imgui_demo for the case of using a custom allocator actually useful?

Yes, as I said in my post, this is a minor thing and not really affecting the imgui usage. But the demos will crash if you want to use them in your app with custom allocators. You could also define them as a global var and clear them on Imgui::Shutdown, if you like the code to be working correctly in every scenario

thanks

[ocornut]

You still aren't explaining which sequence of allocator-assignment/allocation/free is leading to crash.
You are probably suggesting that you are getting into a case of alloc/free allocator mismatch but it would be nice to clarify the ordering of things.

[septag]

You can reproduce the bug by :

• Override the memory allocator on init
• Call ShowTestWindow after each NewFrame
• Open the "Graph Widgets" panel
• Close the app, call ImGui::Shutdown() then destroy the overrided allocator and exit the program

The program will crash because the custom allocator is destroyed, after that, the static ImVector<float> values; dtor is called, which calls the ImGui::MemFree(Data); that points to corrupt data.

[ocornut]

I have fixed this instance of use of ImVector because it's more common to run this code when using the demo. However the Log, Console and Custom Drawing demo are using ImVector<> as well and wouldn't as easily be converted.

Normally you would keep your allocator active so it function during static destruction. I am not sure how you would suggest to fix the other instances of heap use in the demo in way that wouldn't be detrimental to the demo code.

NB: ImGui::Shutdown() doesn't affect anything used by the demo code which is standalone.

[ocornut]

@septag Ping? can you keep your allocator active during static destruction? is using the demo code with your custom allocator a requirement? can you suggest a solution? (see: Log/Console/CustomDrawing demos).

[septag]

can you keep your allocator active during static destruction?

my allocator is a static global itself. but I think the compiler doesn't guarantee which static variable gets destructed first when program exits.

is using the demo code with your custom allocator a requirement?

Not at all. I'm just doing this for testing purposes in my engine. it's not a permanent thing at all. I was just pointing out that It's a bad practice doing these stuff in the ImGui code and may raise problems with some users.

can you suggest a solution?

An easy and convenient way would be adding InitializeDemos (optional) and shutdownDemos functions to the demos API so it do proper init/shutdown.

[ocornut]

my allocator is a static global itself. but I think the compiler doesn't guarantee which static variable gets destructed first when program exits

I suppose the idea of keeping your allocator active during static destruction is that it would NOT have a destructor.

Each of the individual demo is independent and there's no high-level encompassing structure, code/data are as local as possible so that the example as easy to follow. This is also why most examples are using local static variables for storage. To implement ShutdownDemos() correctly I would have to pull out all e.g. static ImGuiTextFilter filter outside their local scope so they can be cleared on demand, that would really affect the locality and readability of the demos.

I would prefer to pass since it isn't a blocking issue, it only affects actively using the demos, we have fixed the most common culprit (the plotting demo that was more common click away) and using custom allocators is really uncommon. If I were to fix it it would alter demo code readability for what is essentially a very rare use case.

If you aren't comfortable with not destroying your allocator and still want to seldomly use the stock log/consoles/drawing demos within your application you can either bear the crash on quitting or skip destroying your allocator only if imgui demos have been used.

[ocornut]

TLDR; you are right but i don't have a satisfying solution without butchering the demo. I'm hoping my suggestions above are good enough of a workaround.