Gnimuc / CImGui.jl

Julia wrapper for cimgui
https://github.com/cimgui/cimgui
MIT License
259 stars 25 forks source link

Memory leak #64

Closed alenskorobogatova closed 3 years ago

alenskorobogatova commented 3 years ago

Memory leak when launching examples of the master branch of CImGui (launch occurs as described in the readme file)

Gnimuc commented 3 years ago

Which example?

sairus7 commented 3 years ago

Confirm memory leak at Windows 10, both from demo/demo.jl and examples/demo.jl.

sairus7 commented 3 years ago

Looks like I didn't notice this leak in this PR, since there is very slow operating memory increase of Julia process. This bug also present in this ImPlot branch.

Looked at both demo.jl code but I have no idea why it happens, maybe because I just commented glfwSetErrorCallback (didn't know what argument it takes) or misused ImGuiGLFWBackend.create_context and ImGuiOpenGLBackend.create_context functions, or there are deeper issues with newly generated code? @Gnimuc can you take a look at this?

Gnimuc commented 3 years ago

only on Windows? does the problem persist if you disable the docking and multi-viewport features?

Gnimuc commented 3 years ago

The leak is probably due to this line in the backend: https://github.com/JuliaImGui/ImGuiGLFWBackend.jl/blob/a005da40908b1d5b1335762ef0723bae570453f8/src/interface.jl#L207

The implementation assumes the C-managed Ptr{ImGuiPlatformMonitor} pointer is initilized as a C_NULL, so there is no need to manually free it on the Julia side before we fill up it with the new resources. I've checked the related IMGUI code before, but I might be wrong as well.

sairus7 commented 3 years ago

I'm not sure if I understand you corectly, do you mean that monitors_ptr ownership is passed to platform_io object that is gc'ed by C library?

Gnimuc commented 3 years ago

Not gc-ed, but free-ed by C/C++. As platform_io is an object managed by the C/C++ library, I guess the C++ code does something like:

if (platform_io.monitors_ptr)
    delete[] platform_io.monitors_ptr

This is why I allocated monitors_ptr through Libc.malloc.

sairus7 commented 3 years ago

But this is called only once - how can it cause constant memory increase inside rendering cycle?

Can it be somehow related to this bug with arrays or pointers? https://github.com/wsphillips/ImPlot.jl/issues/23

Gnimuc commented 3 years ago

By memory leak, do you mean "memory constantly increasing when running the GUI window" or "memory is not released correctly after GUI window is closed". I would like to know whether the leak is on the C side or the Julia side. A MWE example would be helpful here.

sairus7 commented 3 years ago

I mean, memory constantly increasing when running the GUI window. It repeats with most examples, lets take short one from ImGuiGLFWBackend readme, just an empty window with no widgets, no multiport or docking. It starts with 360 MB of RAM and after 1 minute grows up to 1017 MB. (~11MB/sec).


using ImGuiGLFWBackend
using ImGuiGLFWBackend.LibCImGui
using ImGuiGLFWBackend.LibGLFW
using ImGuiOpenGLBackend
using ImGuiOpenGLBackend.ModernGL

# create contexts
imgui_ctx = igCreateContext(C_NULL)

window_ctx = ImGuiGLFWBackend.create_context()
window = ImGuiGLFWBackend.get_window(window_ctx)

gl_ctx = ImGuiOpenGLBackend.create_context()

# enable docking and multi-viewport
# io = igGetIO()
# io.ConfigFlags = unsafe_load(io.ConfigFlags) | ImGuiConfigFlags_DockingEnable
# io.ConfigFlags = unsafe_load(io.ConfigFlags) | ImGuiConfigFlags_ViewportsEnable

# set style
igStyleColorsDark(C_NULL)

# init
ImGuiGLFWBackend.init(window_ctx)
ImGuiOpenGLBackend.init(gl_ctx)

try
    while glfwWindowShouldClose(window) == GLFW_FALSE
        glfwPollEvents()
        # new frame
        ImGuiOpenGLBackend.new_frame(gl_ctx)
        ImGuiGLFWBackend.new_frame(window_ctx)
        igNewFrame()

        # UIs
        # igShowDemoWindow(Ref(true))
        # igShowMetricsWindow(Ref(true))

        # rendering
        igRender()
        glfwMakeContextCurrent(window)
        w_ref, h_ref = Ref{Cint}(0), Ref{Cint}(0)
        glfwGetFramebufferSize(window, w_ref, h_ref)
        display_w, display_h = w_ref[], h_ref[]
        glViewport(0, 0, display_w, display_h)
        glClearColor(0.45, 0.55, 0.60, 1.00)
        glClear(GL_COLOR_BUFFER_BIT)
        ImGuiOpenGLBackend.render(gl_ctx)

        # if unsafe_load(igGetIO().ConfigFlags) & ImGuiConfigFlags_ViewportsEnable == ImGuiConfigFlags_ViewportsEnable
        #     backup_current_context = glfwGetCurrentContext()
        #     igUpdatePlatformWindows()
        #     GC.@preserve gl_ctx igRenderPlatformWindowsDefault(C_NULL, pointer_from_objref(gl_ctx))
        #     glfwMakeContextCurrent(backup_current_context)
        # end

        glfwSwapBuffers(window)
    end
catch e
    @error "Error in renderloop!" exception=e
    Base.show_backtrace(stderr, catch_backtrace())
finally
    ImGuiOpenGLBackend.shutdown(gl_ctx)
    ImGuiGLFWBackend.shutdown(window_ctx)
    igDestroyContext(imgui_ctx)
    glfwDestroyWindow(window)
end
Gnimuc commented 3 years ago

Thanks for the example! I can reproduce it on my machine as well. Trying to looking into it...

sairus7 commented 3 years ago

Some effects:

Gnimuc commented 3 years ago

Thanks for reporting the issue! I've just tagged a new version which should be released within a hour.

sairus7 commented 3 years ago

I've tried the same example with dev ImGuiOpenGLBackend, and now it is not growing in memory but produces a bunch of the following errors from time to time:

┌ Error: GLFW ERROR: code 65544 msg: WGL: Failed to make context current: The handle is invalid.
└ @ ImGuiGLFWBackend C:\Users\user\.julia\packages\ImGuiGLFWBackend\42XtF\src\callbacks.jl:2
┌ Error: GLFW ERROR: code 65544 msg: WGL: Failed to make context current: The requested transformation operation is not supported.
└ @ ImGuiGLFWBackend C:\Users\user\.julia\packages\ImGuiGLFWBackend\42XtF\src\callbacks.jl:2
Gnimuc commented 3 years ago

I can not reproduce that on macOS, feel free to file a new issue.

Does this happen without the new patch? If so, you could apply the patch right above this line:

https://github.com/JuliaImGui/ImGuiOpenGLBackend.jl/blob/master/src/device.jl#L114