Deterministic call to `oni_destroy_ctx()` in clroni

[jonnew]

It seems that, within clroni, ContextHandle.ReleaseHandle() is called by the GC when the reference count goes to 0 marking the Handle as invalid. This means that oni_destroy_ctx() is called non-deterministically. Since SafeHandleZeroOrMinusOneIsInvalid implements IDisposable, we wonder why oni_destroy_ctx() cannot be called deterministically. When is Dispose() called in this case? Is it also linked to the reference count and Handle invalid state?

[jonnew]

This is expected behavior with our current use of SafeHandle. The overridden ReleaseHandle method is what ultimately calls NativeMethods.oni_destroy_ctx(handle). ReleaseHandle is, in turn, called by the a critical finalizer on the finalizer thread prior to garbage collection. This type of finalizer has some special properties that ensure the resources tied to the handle are completely closed and that this closing action occurs exactly 1 time.

SafeHandle also implements IDisposable. However, all Dispose() does is allow resources to be freed. This might not happen immediately if other threads are using the same instance of the safe handle, but will happen as soon as that is no longer the case.

So, this appears to be a cost of doing business with SafeHandle. We loose the ability to know exactly when oni_destory_ctx is called and we gain the ability to ensure that it is called once and to competition.

source: https://learn.microsoft.com/en-us/dotnet/fundamentals/runtime-libraries/system-runtime-interopservices-safehandle

[aacuevas]

Oh, I think I get it.

One thing we could do, if we desired the ability to ensure that calling Dispose forcefully closes the context, would be to implement a class inheriting from SafeHandleZeroOrMinusOneIsInvalid , since Dispose is protected virtual, that closes the context on disposal and then calls the parent dispose to ensure all the proper operations are done.

We need to think thoroughly if this is something we want and what dangers could it entail.

[jonnew]

We in fact already do use SafeHandleZeroOrMinusOneIsInvalid. This is just a SafeHandle with the following added to it

public override bool IsInvalid => handle == IntPtr.Zero || handle == new IntPtr(-1);

All this does is automatically mark the handle as invalid in certain possible situations . e.g. it says, if for whatever reason you managed to become the proud owner of IntPtr.Zero (aka nullptr) because e.g. a file didn't exist or the hardware didn't open then IsInvalid can be used to account for that in later code. It does not affect lifetimes or when ReleaseHandle() is called as far as I understand.

[jonnew]

FWIW, I'm not sure what the consequences will be of e.g. throwing NativeMethods.oni_destroy_ctx(handle) into our dispose call. All the docs say that ReleaseHandle() should contain the native code that is required to free the underlying resource, for example in SafeFileHandle

namespace Microsoft.Win32.SafeHandles
{
    internal sealed class SafeFindHandle : SafeHandleZeroOrMinusOneIsInvalid
    {
        public SafeFindHandle() : base(true) { }

        protected override bool ReleaseHandle()
        {
            return Interop.Kernel32.FindClose(handle);
        }
    }
}

So it really seems like this is the correct practice, I think due to the nature of the critical finalizer.