ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
58.86k stars 10.05k forks source link

Fonts freed twice during shutdown #6781

Closed Beefster09 closed 11 months ago

Beefster09 commented 11 months ago

(you may also go to Demo>About Window, and click "Config/Build Information" to obtain a bunch of detailed information that you can paste here)

Version/Branch of Dear ImGui:

Version: 1.89.8dock

Back-end/Renderer/Compiler/OS

Dear ImGui 1.89.8 (18980)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: _WIN32
define: _WIN64
define: _MSC_VER=1936
define: _MSVC_LANG=201402
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: GLFW
io.BackendRendererName: OpenGL
io.ConfigFlags: 0x00000040
 DockingEnable
io.ConfigViewportsNoDecoration
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigWindowsMoveFromTitleBarOnly
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 11 fonts, Flags: 0x00000000, TexSize: 512,1024
io.DisplaySize: 2309.00,1186.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

My Issue/Question:

It seems that during shutdown, fonts are not being freed properly. While this isn't exactly a critical issue since it happens during shutdown, it could potentially lead to segfaults that prevent clean exit.

Tracing through the code paths of shutdown, it seems that font atlases de-initialize the font vector by free-clearing instead of destructor-clearing. I don't know a whole lot about the guts of the library, but I don't think this is safe to do because fonts have nontrivial destructors.

Note that I am using this from my own fork of odin-imgui, linking statically via cimgui. I have discovered the apparent bad free via a tracking allocator provided in the Odin core library.

Screenshots/Video

My program typically exits with error messages like this:

* X:/Documents/Projects/tool-assisted-samurai-odin/libs/imgui/util.odin(85:12) allocation 7FF6E8227892 was freed badly
* X:/Documents/Projects/tool-assisted-samurai-odin/libs/imgui/util.odin(85:12) allocation 7FF6E84CB778 was freed badly
* X:/Documents/Projects/tool-assisted-samurai-odin/libs/imgui/util.odin(85:12) allocation 7FF6E82477E1 was freed badly
* X:/Documents/Projects/tool-assisted-samurai-odin/libs/imgui/util.odin(85:12) allocation 7FF6E84CB778 was freed badly
* X:/Documents/Projects/tool-assisted-samurai-odin/libs/imgui/util.odin(85:12) allocation 7FF6E8267732 was freed badly
* X:/Documents/Projects/tool-assisted-samurai-odin/libs/imgui/util.odin(85:12) allocation 7FF6E84CB778 was freed badly
* X:/Documents/Projects/tool-assisted-samurai-odin/libs/imgui/util.odin(85:12) allocation 7FF6E8287681 was freed badly

Obviously these come from Odin, not ImGui, but perhaps this behavior can be verified in another program using valgrind or something.

Standalone, minimal, complete and verifiable example:

uhh, this might be kind of hard to come up with since I don't interface directly through the C++

ocornut commented 11 months ago

The first line of ImGui::Shutdown() delete the font atlas:

    if (g.IO.Fonts && g.FontAtlasOwnedByContext)
    {
        g.IO.Fonts->Locked = false;
        IM_DELETE(g.IO.Fonts);
    }

Which call all ClearXXX() functions of ImFontAtlas, including ClearFonts() which calls Fonts.clear_delete().

I also don't know what " was freed badly " means in your log.

Please provide a repro (do you load custom fonts or does it repro without?) and try to look for more details.

Beefster09 commented 11 months ago

I also don't know what " was freed badly " means in your log

It corresponds to a double-free or being freed by a different allocator than what allocated it.

This only started happening when I started loading fonts. The default font doesn't cause this.

I also accept the possibility that this is an programming error on my part, but haven't been able to find solid evidence one way or the other (i.e. what I'm doing wrong). At the very least, I hope to at least leave behind something that other users can learn from in the future.

Beefster09 commented 11 months ago

I figured out the problem:

The memory it was trying to free was embedded font data. I was able to fix it with something akin to:

// ImFontConfig config
config.FontDataOwnedByAtlas = false

before loading fonts

Beefster09 commented 11 months ago

The documentation does not have any information on loading uncompressed fonts embedded in source code (as this is super convenient to do in languages like Odin), so I had no idea this was necessary until I dove into the source code to see a comment on the matter + about 45 minutes in the debugger.

This could be alleviated with some additions to the documentation.

ocornut commented 11 months ago

The header file says:

// - Important: By default, AddFontFromMemoryTTF() takes ownership of the data. Even though we are not writing to it, we will free the pointer on destruction.
//   You can set font_cfg->FontDataOwnedByAtlas=false to keep ownership of your data and it won't be freed,

I am going to add notes about this in https://github.com/ocornut/imgui/blob/master/docs/FONTS.md now. It's a common pitfall and a big API issue ihmo, but I've been waiting until a larger rework to change that behavior. Thank you for the feedback.

ocornut commented 11 months ago

I have pushed this amend to the doc c4dc8fd10190d97fa55055bd3a47381b98e3e9bf Being a common problem I tried to make it quite visible: https://github.com/ocornut/imgui/blob/master/docs/FONTS.md