Memory leak (and crash) in ImGui_ImplAndroid_Shutdown()

[mack-w]

• axmol version: 2.1.2 (I believe it's reproducible on latest git as well)
• devices test on: Android Simulator (hardware and software rendering)
• developing environments
  • NDK version: r23c
  • cmake version: 3.22.1
  • Android API Level: 34

Steps to Reproduce:

1. Download latest Android Studio and Axmol, build the CppTests project
2. Observe how the test app crashes every time during Autotest, or when manually exiting ImGui test

I'm completely new to developing with Axmol (and any sort of game development in general), so forgive me if I made technical mistakes. I have also searched the whole Axmol repo and omar's ImGui repo as well, so apologies if this issue was a duplicate.

I've been trying to get Axmol and Android Studio working this afternoon, and when I compile the CppTests project, I notice that the app crashes immediately if I try to conduct an Autotest, or when I leave the ImGui tests. Everything else works just fine. When I look at the logs, I notice that an assertion from ImGui::Shutdown(), ASSERT_USER_ERROR(g.IO.BackendRendererUserData == NULL) failed. I was able to locate the code which guarantees that the other assertion is good, in ImGui_ImplAndroid_Shutdown().

To my surprise, the way that this piece of code does resource de-allocation astonishes me: it simply sets pointers from struct ImGuiIO to NULL. This is, to my knowledge, not in accordance with standard C++ coding practices to the very least. By removing the assertion in ImGui::Shutdown and watching the memory in LLDB, I was able to prove that the contents that the pointer points to retains their lifecycle for a long time after the ImGui Scene has been destroyed. This was further proved by setting AX_REF_LEAK_DETECTION as 1 and printing the leaked objects. By repeating ImGuiTest() in an infinite loop, I observed ~2 MiB memory leaked in about 30 minutes, loosely corresponding to about 100 bytes, or an ImGui frame leaked every second.

I admit that this may not be a major issue, but hopes that it may get fixed soon. At least the app should not crash on destroying the ImGui Scene, that would make games (which require extensive configuration) based on Axmol impossible to play.

[rh101]

The ImGui implementation for Android was a test implementation to try and see if ImGui would work on that platform, since it was previously limited to desktop (Windows/OpenGL). The input handling in the Android implementation of ImGui may also not be working correctly, so there is little point in using it on that platform for the time being.

It would be best to disable ImGui if you're building a project (including cpp-tests) for Android, at least until all these issues with the Android ImGui implementation are addressed.

[aismann]

@rh101
I added your text on the Wiki page:
https://github.com/axmolengine/axmol/wiki/Extensions#extension-specific-notes

[aismann]

@halx99
Please add helpdesk or wiki

[rh101]

I've been trying to get Axmol and Android Studio working this afternoon, and when I compile the CppTests project, I notice that the app crashes immediately if I try to conduct an Autotest, or when I leave the ImGui tests.

Out of curiosity, for what reason are you running the Auto Test function? If you're new to Axmol, and you're looking to learn more about it, then just be aware that the Auto Test is not for that purpose.

[mack-w]

I've been trying to get Axmol and Android Studio working this afternoon, and when I compile the CppTests project, I notice that the app crashes immediately if I try to conduct an Autotest, or when I leave the ImGui tests.

Out of curiosity, for what reason are you running the Auto Test function? If you're new to Axmol, and you're looking to learn more about it, then just be aware that the Auto Test is not for that purpose.

I was just tryin' make sure that everything works absolutely fine. I was playing with cocos2d this morning and found that it sucks... dependency hell. That's when I somehow spotted Axmol in some issue in their repository.

[rh101]

PR #1920 will fix the ImGui crash on Android, but as for the possible memory leak, that will require more investigation.

[rh101]

To my surprise, the way that this piece of code does resource de-allocation astonishes me: it simply sets pointers from struct ImGuiIO to NULL. This is, to my knowledge, not in accordance with standard C++ coding practices to the very least.

io.BackendPlatformUserData and io.BackendRendererUserData both hold the same pointer, which is the value returned from ImGui_ImplAndroid_GetBackendData();, and that pointer is deleted, as you can see at the end of the ImGui_ImplAndroid_Shutdown() function:

void ImGui_ImplAndroid_Shutdown()
{
    ImGui_ImplAndroid_Data* bd = ImGui_ImplAndroid_GetBackendData();
...
    io.BackendPlatformUserData = NULL;
    io.BackendRendererUserData = NULL;

    IM_DELETE(bd);
...
}

The pointer is an instance of ImGui_ImplAndroid_Data, which does not inherit from ax::Object/ax::Ref, so it has nothing to do with the reference tracking in Axmol. This means anything you see from AX_REF_LEAK_DETECTION is not from the allocation of ImGui_ImplAndroid_Data, but perhaps something else linked to it, or entirely unrelated to it.

The only object that is retained is the EventListenerTouchOneByOne, but that is also correctly released correctly in the ImGui_ImplAndroid_Shutdown() function.

[mack-w]

io.BackendPlatformUserData and io.BackendRendererUserData both hold the same pointer, which is the value returned from ImGui_ImplAndroid_GetBackendData();, and that pointer is deleted, as you can see at the end of the ImGui_ImplAndroid_Shutdown() function:

void ImGui_ImplAndroid_Shutdown()
{
    ImGui_ImplAndroid_Data* bd = ImGui_ImplAndroid_GetBackendData();
...
    io.BackendPlatformUserData = NULL;
    io.BackendRendererUserData = NULL;

    IM_DELETE(bd);
...
}

Yeah I see, but as far as I knows what IM_DELETE does is simply calls on the destructor (in this case there's none) and then frees the memory via library call. Not sure whether there're objects leaked in this process… I haven't fully investigated whether all classes of objects pointed to inside the ImGui_ImplAndroid_Data are indeed inherited from ax::Ref and are placed on the autorelease list

[rh101]

Yeah I see, but as far as I knows what IM_DELETE does is simply calls on the destructor (in this case there's none) and then frees the memory via library call. Not sure whether there're objects leaked in this process…

The ImGui_ImplAndroid_Data is being freed correctly.

I haven't fully investigated whether all classes of objects pointed to inside the ImGui_ImplAndroid_Data are indeed inherited from ax::Ref and are placed on the autorelease list

Autorelease doesn't actually have anything to do with this, and there may be some misunderstanding as to what its purpose is. Just in case, we do have a wiki article explaining the reference counting model used in Axmol, along with a section on autorelease, which you can find here.