Use SafeHandles

Author: bording

LibGit2Sharp/Core/Handles/Libgit2Object.cs

@@ -1,31 +1,31 @@
 // This activates a lightweight mode which will help put under the light
-// incorrectly released handles by outputing a warning message in the console.
+// incorrectly released handles by outputting a warning message in the console.
 //
 // This should be activated when tests are being run on the CI server.
 //
 // Uncomment the line below or add a conditional symbol to activate this mode
 
-// #define LEAKS_IDENTIFYING
+//#define LEAKS_IDENTIFYING
 
-// This activates a more throrough mode which will show the stack trace of the
+// This activates a more thorough mode which will show the stack trace of the
 // allocation code path for each handle that has been improperly released.
 //
 // This should be manually activated when some warnings have been raised as
 // a result of LEAKS_IDENTIFYING mode activation.
 //
 // Uncomment the line below or add a conditional symbol to activate this mode
 
-// #define LEAKS_TRACKING
+//#define LEAKS_TRACKING
 
 using System;
-using System.Linq;
-using System.Diagnostics;
-using System.Globalization;
-using System.Collections.Generic;
+using Microsoft.Win32.SafeHandles;
 
 #if LEAKS_IDENTIFYING
 namespace LibGit2Sharp.Core
 {
+    using System.Collections.Generic;
+    using System.Linq;
+
     /// <summary>
     /// Holds leaked handle type names reported by <see cref="Core.Handles.Libgit2Object"/>
     /// </summary>
@@ -78,30 +78,27 @@ public static IEnumerable<string> TypeNames
 
 namespace LibGit2Sharp.Core.Handles
 {
-    internal unsafe abstract class Libgit2Object : IDisposable
+#if LEAKS_TRACKING
+    using System.Diagnostics;
+    using System.Globalization;
+#endif
+
+    internal unsafe abstract class Libgit2Object : SafeHandleZeroOrMinusOneIsInvalid
     {
 #if LEAKS_TRACKING
         private readonly string trace;
         private readonly Guid id;
 #endif
 
-        protected void* ptr;
-
-        internal void* Handle
+        internal unsafe Libgit2Object(void* ptr, bool owned)
+            : this(new IntPtr(ptr), owned)
         {
-            get
-            {
-                return ptr;
-            }
         }
 
-        bool owned;
-        bool disposed;
-
-        internal unsafe Libgit2Object(void* handle, bool owned)
+        internal unsafe Libgit2Object(IntPtr ptr, bool owned)
+            : base(owned)
         {
-            this.ptr = handle;
-            this.owned = owned;
+            SetHandle(ptr);
 
 #if LEAKS_TRACKING
             id = Guid.NewGuid();
@@ -111,53 +108,20 @@ internal unsafe Libgit2Object(void* handle, bool owned)
 #endif
         }
 
-        internal unsafe Libgit2Object(IntPtr ptr, bool owned)
-            : this(ptr.ToPointer(), owned)
-        {
-        }
+        internal IntPtr AsIntPtr() => DangerousGetHandle();
 
-        ~Libgit2Object()
-        {
-            Dispose(false);
-        }
-
-        internal bool IsNull
-        {
-            get
-            {
-                return ptr == null;
-            }
-        }
-
-        internal IntPtr AsIntPtr()
-        {
-            return new IntPtr(ptr);
-        }
-
-        public abstract void Free();
-
-        void Dispose(bool disposing)
+        protected override void Dispose(bool disposing)
         {
 #if LEAKS_IDENTIFYING
-            bool leaked = !disposing && ptr != null;
+            bool leaked = !disposing && DangerousGetHandle() != IntPtr.Zero;
 
             if (leaked)
             {
                 LeaksContainer.Add(GetType().Name);
             }
 #endif
 
-            if (!disposed)
-            {
-                if (owned)
-                {
-                    Free();
-                }
-
-                ptr = null;
-            }
-
-            disposed = true;
+            base.Dispose(disposing);
 
 #if LEAKS_TRACKING
             if (!leaked)
@@ -172,11 +136,6 @@ void Dispose(bool disposing)
             }
 #endif
         }
-
-            public void Dispose()
-        {
-            Dispose(true);
-        }
     }
 }
 

LibGit2Sharp/Core/Handles/Objects.cs

@@ -82,14 +82,16 @@ for (var i = 0; i < cNames.Length; i++)
         {
         }
 
-        public override void Free()
+        protected override bool ReleaseHandle()
         {
-            NativeMethods.<#= cNames[i] #>_free((<#= cNames[i] #>*) ptr);
+            NativeMethods.<#= cNames[i] #>_free((<#= cNames[i] #>*)DangerousGetHandle());
+
+            return true;
         }
 
         public static implicit operator <#= cNames[i] #>*(<#= csNames[i] #> handle)
         {
-            return (<#= cNames[i] #>*) handle.Handle;
+            return (<#= cNames[i] #>*)handle.DangerousGetHandle();
         }
     }
 

LibGit2Sharp/Core/Handles/Objects.tt

@@ -216,11 +216,11 @@ private IEnumerable<Reference> ListReferencesInternal(string url, CredentialsHan
 
         static RemoteHandle BuildRemoteHandle(RepositoryHandle repoHandle, string url)
         {
-            Debug.Assert(repoHandle != null && !repoHandle.IsNull);
+            Debug.Assert(repoHandle != null && !repoHandle.IsInvalid);
             Debug.Assert(url != null);
 
             RemoteHandle remoteHandle = Proxy.git_remote_create_anonymous(repoHandle, url);
-            Debug.Assert(remoteHandle != null && !(remoteHandle.IsNull));
+            Debug.Assert(remoteHandle != null && !remoteHandle.IsInvalid);
 
             return remoteHandle;
         }

LibGit2Sharp/Network.cs

@@ -35,7 +35,7 @@ private protected Reference(IRepository repo, string canonicalName, string targe
         // This overload lets public-facing methods avoid having to use the pointers directly
         internal static unsafe T BuildFromPtr<T>(ReferenceHandle handle, Repository repo) where T : Reference
         {
-            return BuildFromPtr<T>((git_reference*) handle.Handle, repo);
+            return BuildFromPtr<T>((git_reference*)handle.AsIntPtr(), repo);
         }
 
         internal static unsafe T BuildFromPtr<T>(git_reference* handle, Repository repo) where T : Reference

LibGit2Sharp/Reference.cs

@@ -575,7 +575,7 @@ internal GitObject LookupInternal(ObjectId id, GitObjectType type, string knownP
 
             using (ObjectHandle obj = Proxy.git_object_lookup(handle, id, type))
             {
-                if (obj == null || obj.IsNull)
+                if (obj == null || obj.IsInvalid)
                 {
                     return null;
                 }
@@ -1781,7 +1781,7 @@ public void RevParse(string revision, out Reference reference, out GitObject obj
             using (var objH = handles.Item1)
             using (var refH = handles.Item2)
             {
-                reference = refH.IsNull ? null : Reference.BuildFromPtr<Reference>(refH, this);
+                reference = refH.IsInvalid ? null : Reference.BuildFromPtr<Reference>(refH, this);
                 obj = GitObject.BuildFrom(this, Proxy.git_object_id(objH), Proxy.git_object_type(objH), PathFromRevparseSpec(revision));
             }
         }