mono / CppSharp

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

Finalizers #148

Closed tomspilman closed 10 years ago

tomspilman commented 10 years ago

It would be nice for CppSharp to generate C# finalizers for all wrapped objects. I currently work around this by adding them to a separate source file as a partial class implementation:

    public unsafe partial class Texture
    {
        ~Texture()
        {
            Dispose(false);
        }
    }

    public unsafe partial class RenderTarget
    {
        ~RenderTarget()
        {
            Dispose(false);
        }
    }

The finalizer is necessary to properly cleanup the unmanaged parts of the wrapper when the user omits a direct call to Dispose(). You already call GC.SuppressFinalize(this) which means the finalizer will not do any extra work unless it is needed.

Some people prefer to not have finalizers and instead force the caller to properly dispose of their objects. In this case adding a finalizer could be optional and when disabled it could add something like this instead:

#if DEBUG
    ~WrapperClass()
    {
        Debug.Fail("You forgot to call Dispose!");
    }
#endif

To alert the user that they forgot to Dispose() this object and that the unmanaged objects were leaked.

By the way we're using CppSharp to generate C# wrappers for a project running on the PS4. It has been fantastic and much easier than if we needed to generate our own bindings.

tritao commented 10 years ago

I took a first stab at this, let me know if it works for you. We can easily add support for the #ifdef version if it's working alright.

Thank you for the feedback, glad to know that CppSharp's working out fine for you :+1:

What kind of project are building? If it's not public, let me know about once you ship it, I'd like to try it.

tomspilman commented 10 years ago

Thanks! I'll try this out today and get you some feedback.

What kind of project are building?

We're working on the PS4 port of TowerFall.

tomspilman commented 10 years ago

@tritao - So the finalizers work, but there is a complication.

We have a manager class that returns a state object like this:

        public static ObjectState GetState(int idx)
        {
            var __ret = Internal.GetState_0(idx);
            if (__ret == global::System.IntPtr.Zero) return null;
            return new ObjectState (__ret);
        }

In this call the __ret object was allocated by the C++ code using its own internal allocator. This is an object that shouldn't be freed using Marshal.FreeHGlobal. Before as long as someone didn't call Dispose on it... it was fine... although if they did they would get a crash.

With finalizers in place it will always call Marshal.FreeHGlobal eventually.

The issue here seems to be one of memory ownership. If the C++ code returns a pointer to the C# code... the C# code shouldn't assume ownership and try to free it.

This of course can get really complicated.

tomspilman commented 10 years ago

Hum... maybe i'm thinking about this incorrect. As long as Marshal.FreeHGlobal is using the normal malloc/free heap internally then C# can freely destroy memory allocated by C++.

So it has nothing to do with memory really... It is all about if the object has a public constructor and if it is disposable or not.

In my case I could suppress the dispose/finalize on certain types that shouldn't be explicitly managed like that.

tritao commented 10 years ago

Yes, destroying memory allocated by C++ should work, but sometimes we do not want to deallocate if C++ has memory ownership for those objects.

I was thinking we add a boolean to the constructor that takes a pointer to a native object, to flag who owns the memory. We can also have some standardized attribute macro to easily specify this in the native headers.

tomspilman commented 10 years ago

I was thinking we add a boolean to the constructor that takes a pointer to a native object, to flag who owns the memory.

Yea... that is a pretty common pattern I've seen used in other wrapper generators.