mono / CppSharp

Tools and libraries to glue C/C++ APIs to high-level languages
MIT License
3.03k stars 497 forks source link

Memory leak of default value arguments #1370

Open firesgc opened 4 years ago

firesgc commented 4 years ago
Brief Description

Hello!

I'm generating C# bindings for ImGUI and have the problem of memory leaks of default argument values.

Here is an example:

Used headers
    // c++ defintion
    IMGUI_API bool BeginChild(ImGuiID id, const ImVec2& size = ImVec2(0,0), bool border = false, ImGuiWindowFlags flags = 0);

    // c#: function for call BeginChild() with default arguments
    public static bool BeginChild(string str_id)
    {
        return BeginChild(str_id, new global::ImGUI.ImVec2(0, 0), false, 0);
    }

    // c#: constructor of default argument
    public ImVec2(float _x, float _y)
    {
        __Instance = Marshal.AllocHGlobal(sizeof(global::ImGUI.ImVec2.__Internal))
        __ownsNativeInstance = true;
        NativeToManagedMap[__Instance] = this;
        __Internal.ctor(__Instance, _x, _y);
    }

In the C# function BeginChild() an ImGUI.ImVec2 is created, but its Dispose function is never called. Shouldn't temporary objects in such functions be released directly? Or is there an option to free the memory "somehow"?

Used settings

options.GenerateDefaultValuesForArguments = true; options.GenerateFinalizers = true; Target: MSVC OS: Windows 10

ddobrev commented 4 years ago

@firesgc thank you for your report. We've known about this problem but it's really difficult to think of a way to free the default parameter while at the same time preserving a convenient API. Our current plan is to implement the standard IDisposable pattern on all generated classes with the addition of calling the native destructor in the finalizer if necessary. The only issue and therefore reason not to have done it yet is that we have a mapping between native and managed objects in order to ensure the same wrapper is used for the same native object. This map is a dictionary which holds objects alive. One possible solution is weak references but they might release the object when it's still needed. But the bottom-line is that if we had this solution in place, it would solve your problem too.

firesgc commented 4 years ago

Thank you very much indeed! Do you think this will be added in the near future? If not, I will disable the default parameter generation and do it manually.

ddobrev commented 4 years ago

@firesgc I cannot promise any deadline at present. If the matter's urgent, you might be better off disabling them.