Skip to content
This repository has been archived by the owner on Mar 30, 2019. It is now read-only.

Finalizer releases last ref too early when EnableReleaseOnFinalizer is true #1106

Open
MillerJames opened this issue Nov 26, 2018 · 2 comments

Comments

@MillerJames
Copy link

MillerJames commented Nov 26, 2018

I’ve found that with Configuration.EnableReleaseOnFinalizer set to true, I occasionally get an exception if the last use of a ComObject is passing it into a generated interop method. For example, when using SharpDX versions 3.1.1 or 4.2.0, the following call to d2drendertarget.FillGeometry occasionally throws an exception. I believe this is because the finalizer runs and decrements the ref count to zero before the native method acquires its own ref:

// Some managed method in my C# code
public void FillGeo(MyGeometryType geometry, D2D.Brush d2dbrush)
{
    D2D.Geometry d2dgeometry = geometry.CreateD2DGeometry();
    this.d2drendertarget.FillGeometry(d2dgeometry, d2dbrush);
}
// generated FillGeometry interop method in SharpDX (comments mine)
public unsafe void DrawGeometry(SharpDX.Direct2D1.Geometry geometry, SharpDX.Direct2D1.Brush brush,
    System.Single strokeWidth, SharpDX.Direct2D1.StrokeStyle strokeStyle)
{
    System.IntPtr geometry_ = System.IntPtr.Zero;
    System.IntPtr brush_ = System.IntPtr.Zero;
    System.IntPtr strokeStyle_ = System.IntPtr.Zero;
    geometry_ = SharpDX.CppObject.ToCallbackPtr<SharpDX.Direct2D1.Geometry>(geometry); 

    // At this point, “geometry” is eligible for finalization, its last use was in the above line. 
    // If the finalizer runs now, it will decrement the native Geometry object's ref count to zero
    brush_ = SharpDX.CppObject.ToCallbackPtr<SharpDX.Direct2D1.Brush>(brush);
    strokeStyle_ = SharpDX.CppObject.ToCallbackPtr<SharpDX.Direct2D1.StrokeStyle>(strokeStyle);

    // The native method called here will try to dereference "geometry_" but the native object it 
    // points at might have been destroyed (if the managed object's finalizer already ran)
    SharpDX.Direct2D1.LocalInterop.CalliStdCallvoid(this._nativePointer, (void *)geometry_, 
        (void *)brush_, strokeWidth, (void *)strokeStyle_, (*(void ***)this._nativePointer)[22]);
}

This case can be trivially resolved with a using block in my code, but it seems like the point of Configuration.EnableReleaseOnFinalizer is that explicitly disposing is not required. I'm working around this issue with using blocks and GC.KeepAlive for the cases I've found in my codebase, but resolving this more generally would be great.

Would it be reasonable to have SharpGen generate GC.KeepAlive calls to delay finalization for objects whose native pointer is passed into a native method call? i.e. the generated method would look something like:

public unsafe void DrawGeometry(SharpDX.Direct2D1.Geometry geometry, SharpDX.Direct2D1.Brush brush, System.Single strokeWidth, SharpDX.Direct2D1.StrokeStyle strokeStyle)
{
    System.IntPtr geometry_ = System.IntPtr.Zero;
    System.IntPtr brush_ = System.IntPtr.Zero;
    System.IntPtr strokeStyle_ = System.IntPtr.Zero;
    geometry_ = SharpDX.CppObject.ToCallbackPtr<SharpDX.Direct2D1.Geometry>(geometry);
    brush_ = SharpDX.CppObject.ToCallbackPtr<SharpDX.Direct2D1.Brush>(brush);
    strokeStyle_ = SharpDX.CppObject.ToCallbackPtr<SharpDX.Direct2D1.StrokeStyle>(strokeStyle);
    SharpDX.Direct2D1.LocalInterop.CalliStdCallvoid(this._nativePointer, (void *)geometry_, 
        (void *)brush_, strokeWidth, (void *)strokeStyle_, (*(void ***)this._nativePointer)[22]);

    GC.KeepAlive(this);
    GC.KeepAlive(geometry);
    GC.KeepAlive(brush);
    GC.KeepAlive(strokeStyle);
}
@jkoritzinsky
Copy link
Contributor

Hey can you open an issue for this on the SharpGenTools repo? That's where the code generator lives. I'm busy with work right now but I'd definitely be interested in adding this into the generator.

@MillerJames
Copy link
Author

Sure, I just opened CppObject finalized before native method is called in SharpGenTools, basically just copy-pasted this issue. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants