smourier / DirectNAot

DirectN AOT compatible version. Only for .NET Core 8 and beyond. Interop Code for .NET Framework, .NET Core and .NET 5+ : DXGI, WIC, DirectX 9 to 12, Direct2D, Direct Write, Direct Composition, Media Foundation, WASAPI, CodecAPI, GDI, Spatial Audio, DVD, Windows Media Player, UWP DXInterop, WinUI3, etc.
MIT License
45 stars 1 forks source link

Memory leak in callback #3

Closed Ollienator closed 1 month ago

Ollienator commented 2 months ago

Hi there,

I am playing around with your AOT version of DirectN and I encountered a problem when using the IMFSourceReaderCallback callback to get frames from a usb web cam.

I implemented the callback public HRESULT OnReadSample(HRESULT hrStatus, uint dwStreamIndex, uint dwStreamFlags, long llTimestamp, IMFSample sample), but I cant find a way to release the sample in this callback after processing it.

In the non-AOT version of DirectN i would just call Marshal.ReleaseComObject(sample); in the callback and all is fine, but the ReleaseComObject method doesnt seem to work for the generated COM bindings in AOT.

Is this a problem of the generated bindings or am I missing something fundemental here?

Thanks in advance, O.

smourier commented 2 months ago

Hi,

As explained here https://github.com/smourier/DirectNAot, DirectNAot design is completely driven by newer .NET COM-interop principles that are defined by the use of source-generated ComWrappers and disabled runtime marshaling that are necessary to support AOT publication.

It means many things have changed and Marshal.ReleaseComObject doesn't work (has no effect) on objects like IMFSample. You'll have to use the newer ComObject.FinalRelease Method instead.

Note this method's current implementation has a nasty bug explained here https://github.com/dotnet/runtime/issues/96901 that can cause crashes on finalization or disposal depending on how/when it's called. It's fixed but I don't think it's fixed with .NET 8.

So, the easiest is to use DirectN's COM utilities and extensions. It provides a ComObject wrapper, similar to what you had with DirectN, so you can do something like this:

HRESULT OnReadSample(HRESULT hrStatus, uint dwStreamIndex, uint dwStreamFlags, long llTimestamp, IMFSample sample)
{
    using var wrapped = new ComObject<IMFSample>(sample);
   // Dispose will call ComObject.FinalRelease() in a safe way https://github.com/smourier/DirectNAot/blob/main/DirectN.Extensions/Com/ComExtensions.cs#L5
}
Ollienator commented 1 month ago

Hi, thanks for the quick reply. I have both tried wrapping the IMFSample into a ComObject as you suggested and also calling ComObject.FinalRelease() directly, but the problem is still there.

I have stepped through the ComObject's disposal in the debugger and I noticed the comment // note: only works on unique instance objects in the ComExtensions.FinalRelease() method. The UniqueInstance property of the IMFSample instance passed into the callback is false, so it does not get released there.

The bug you mentioned seems to be fixed in .NET 9, but I am not seeing any crashes with your implementation on .NET 8, just memory consumption ramping up gigabytes within seconds.

smourier commented 1 month ago

You can try to redefine it manually like this:

[PreserveSig]
[return: MarshalAs(UnmanagedType.Error)]
HRESULT OnReadSample(HRESULT hrStatus, uint dwStreamIndex, uint dwStreamFlags, long llTimestamp, [MarshalUsing(typeof(UniqueComInterfaceMarshaller<IMFSample>))] IMFSample? pSample);

It should force the system to wrap it as a unique instance. If that works, I'll have to modify the generator to have the generated code like this, for all [in] parameters (like it's already done for [out] parameters)

Ollienator commented 1 month ago

Hi,

I have changed the signature as you suggested, rebuilt the libraries and the problem is gone. Thank you for your help fixing this, O.

smourier commented 1 month ago

I've changed this on all COM-object-type parameters https://github.com/smourier/DirectNAot/commit/11bc8c9a7849206d53123ae0b7dbf802be8ee4ed