Deterministic call to oni_destroy_ctx() in clroni #42

Activity

1. jonnew

   self-assigned this

   on Jun 2, 2025
2. jonnew

   added this to the 4.4.0 milestone on Jun 23, 2025
3. jonnew

   assigned

   aacuevas

   on Jun 23, 2025
4. jonnew commented on Jun 24, 2025

   

   jonnew

   on Jun 24, 2025

   MemberAuthor

   More actions

   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
5. aacuevas commented on Jun 24, 2025

   

   aacuevas

   on Jun 24, 2025

   Collaborator

   More actions

   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.
6. jonnew commented on Jun 24, 2025

   

   jonnew

   on Jun 24, 2025

   Last edited by jonnew

   MemberAuthor

   More actions

   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.
7. jonnew commented on Jun 24, 2025

   

   jonnew

   on Jun 24, 2025

   Last edited by jonnew

   MemberAuthor

   More actions

   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.