Better safehandles

LibGit2Sharp/BlameHunk.cs

@@ -2,6 +2,7 @@
 using System.Diagnostics;
 using System.Globalization;
 using LibGit2Sharp.Core;
+using LibGit2Sharp.Core.Handles;
 
 namespace LibGit2Sharp
 {
@@ -42,12 +43,18 @@ internal unsafe BlameHunk(IRepository repository, git_blame_hunk* rawHunk)
             // Signature objects need to have ownership of their native pointers
             if (rawHunk->final_signature != null)
             {
-                FinalSignature = new Signature(rawHunk->final_signature);
+                var ptr = new IntPtr(rawHunk->final_signature);
+                var signatureHandle = new SignatureHandle(ptr, false);
+
+                FinalSignature = new Signature(signatureHandle);
             }
 
             if (rawHunk->orig_signature != null)
             {
-                InitialSignature = new Signature(rawHunk->orig_signature);
+                var ptr = new IntPtr(rawHunk->orig_signature);
+                var signatureHandle = new SignatureHandle(ptr, false);
+
+                InitialSignature = new Signature(signatureHandle);
             }
         }
 

LibGit2Sharp/Configuration.cs

@@ -52,9 +52,9 @@ internal Configuration(
         private void Init(Repository repository)
         {
             configHandle = Proxy.git_config_new();
-            RepositoryHandle repoHandle = (repository != null) ? repository.Handle : null;
+            RepositoryHandle repoHandle = repository?.Handle ?? new RepositoryHandle();
 
-            if (repoHandle != null)
+            if (!repoHandle.IsInvalid)
             {
                 //TODO: push back this logic into libgit2.
                 // As stated by @carlosmn "having a helper function to load the defaults and then allowing you

LibGit2Sharp/Core/GitFilter.cs

@@ -118,7 +118,7 @@ public delegate int git_filter_stream_fn(
     [StructLayout(LayoutKind.Sequential)]
     internal unsafe struct git_filter_source
     {
-        public git_repository* repository;
+        public IntPtr repository;
 
         public char* path;
 

LibGit2Sharp/Core/Handles/Libgit2Object.cs

@@ -18,7 +18,7 @@
 //#define LEAKS_TRACKING
 
 using System;
-using Microsoft.Win32.SafeHandles;
+using System.Runtime.InteropServices;
 
 #if LEAKS_IDENTIFYING
 namespace LibGit2Sharp.Core
@@ -83,20 +83,20 @@ namespace LibGit2Sharp.Core.Handles
     using System.Globalization;
 #endif
 
-    internal unsafe abstract class Libgit2Object : SafeHandleZeroOrMinusOneIsInvalid
+    internal unsafe abstract class Libgit2Object : SafeHandle
     {
 #if LEAKS_TRACKING
         private readonly string trace;
         private readonly Guid id;
 #endif
 
-        internal unsafe Libgit2Object(void* ptr, bool owned)
-            : this(new IntPtr(ptr), owned)
+        internal unsafe Libgit2Object()
+            : base(IntPtr.Zero, true)
         {
         }
 
         internal unsafe Libgit2Object(IntPtr ptr, bool owned)
-            : base(owned)
+            : base(IntPtr.Zero, owned)
         {
             SetHandle(ptr);
 
@@ -108,12 +108,12 @@ internal unsafe Libgit2Object(IntPtr ptr, bool owned)
 #endif
         }
 
-        internal IntPtr AsIntPtr() => DangerousGetHandle();
+        public override bool IsInvalid => handle == IntPtr.Zero;
 
         protected override void Dispose(bool disposing)
         {
 #if LEAKS_IDENTIFYING
-            bool leaked = !disposing && DangerousGetHandle() != IntPtr.Zero;
+            bool leaked = !disposing && !IsInvalid;
 
             if (leaked)
             {

LibGit2Sharp/Core/Handles/Objects.tt

@@ -72,8 +72,8 @@ for (var i = 0; i < cNames.Length; i++)
 #>
     internal unsafe class <#= csNames[i] #> : Libgit2Object
     {
-        internal <#= csNames[i] #>(<#= cNames[i] #>* ptr, bool owned)
-            : base(ptr, owned)
+        internal <#= csNames[i] #>()
+            : base()
         {
         }
 
@@ -84,15 +84,10 @@ for (var i = 0; i < cNames.Length; i++)
 
         protected override bool ReleaseHandle()
         {
-            NativeMethods.<#= cNames[i] #>_free((<#= cNames[i] #>*)AsIntPtr());
+            NativeMethods.<#= cNames[i] #>_free(handle);
 
             return true;
         }
-
-        public static implicit operator <#= cNames[i] #>*(<#= csNames[i] #> handle)
-        {
-            return (<#= cNames[i] #>*)handle.AsIntPtr();
-        }
     }
 
 <#

LibGit2Sharp/Core/Handles/UnownedTreeEntryHandle.cs

@@ -0,0 +1,16 @@
+using System;
+
+namespace LibGit2Sharp.Core.Handles;
+
+internal unsafe class UnownedTreeEntryHandle : TreeEntryHandle
+{
+    internal UnownedTreeEntryHandle()
+        : base(IntPtr.Zero, false)
+    {
+    }
+
+    internal UnownedTreeEntryHandle(IntPtr ptr)
+        : base(ptr, false)
+    {
+    }
+}

LibGit2Sharp/Core/ObjectSafeWrapper.cs

@@ -13,7 +13,7 @@ public unsafe ObjectSafeWrapper(ObjectId id, RepositoryHandle handle, bool allow
 
             if (allowNullObjectId && id == null)
             {
-                objectPtr = new ObjectHandle(null, false);
+                objectPtr = new ObjectHandle(IntPtr.Zero, false);
             }
             else
             {

LibGit2Sharp/FilterSource.cs

@@ -18,11 +18,12 @@ internal unsafe FilterSource(FilePath path, FilterMode mode, git_filter_source*
             SourceMode = mode;
             ObjectId = ObjectId.BuildFromPtr(&source->oid);
             Path = path.Native;
-            Root = Proxy.git_repository_workdir(new IntPtr(source->repository)).Native;
+
+            Root = Proxy.git_repository_workdir(source->repository).Native;
         }
 
         /// <summary>
-        /// Take an unmanaged pointer and convert it to filter source callback paramater
+        /// Take an unmanaged pointer and convert it to filter source callback parameter.
         /// </summary>
         /// <param name="ptr"></param>
         /// <returns></returns>
@@ -32,7 +33,7 @@ internal static unsafe FilterSource FromNativePtr(IntPtr ptr)
         }
 
         /// <summary>
-        /// Take an unmanaged pointer and convert it to filter source callback paramater
+        /// Take an unmanaged pointer and convert it to filter source callback parameter.
         /// </summary>
         /// <param name="ptr"></param>
         /// <returns></returns>

LibGit2Sharp/Identity.cs

@@ -1,4 +1,5 @@
-using LibGit2Sharp.Core;
+using System;
+using LibGit2Sharp.Core;
 using LibGit2Sharp.Core.Handles;
 
 namespace LibGit2Sharp
@@ -52,7 +53,7 @@ internal SignatureHandle BuildNowSignatureHandle()
     internal static class IdentityHelpers
     {
         /// <summary>
-        /// Build the handle for the Indentity with the current time, or return a handle
+        /// Build the handle for the Identity with the current time, or return a handle
         /// to an empty signature.
         /// </summary>
         /// <param name="identity"></param>
@@ -61,7 +62,7 @@ public static unsafe SignatureHandle SafeBuildNowSignatureHandle(this Identity i
         {
             if (identity == null)
             {
-                return new SignatureHandle(null, false);
+                return new SignatureHandle(IntPtr.Zero, false);
             }
 
             return identity.BuildNowSignatureHandle();

LibGit2Sharp/Rebase.cs

@@ -1,7 +1,6 @@
 using System;
 using LibGit2Sharp.Core;
 using LibGit2Sharp.Core.Handles;
-using System.Globalization;
 
 namespace LibGit2Sharp
 {
@@ -65,7 +64,7 @@ internal Rebase(Repository repo)
         unsafe AnnotatedCommitHandle AnnotatedCommitHandleFromRefHandle(ReferenceHandle refHandle)
         {
             return (refHandle == null) ?
-                new AnnotatedCommitHandle(null, false) :
+                new AnnotatedCommitHandle(IntPtr.Zero, false) :
                 Proxy.git_annotated_commit_from_ref(this.repository.Handle, refHandle);
         }
 

LibGit2Sharp/RefSpec.cs

@@ -2,7 +2,6 @@
 using System.Diagnostics;
 using System.Globalization;
 using LibGit2Sharp.Core;
-using LibGit2Sharp.Core.Handles;
 
 namespace LibGit2Sharp
 {
@@ -18,10 +17,10 @@ public class RefSpec
 #pragma warning restore 0414
         readonly IntPtr handle;
 
-        internal unsafe RefSpec(Remote remote, git_refspec* handle)
+        internal unsafe RefSpec(Remote remote, nint handle)
         {
             this.remote = remote;
-            this.handle = new IntPtr(handle);
+            this.handle = handle;
         }
 
         /// <summary>
@@ -37,7 +36,7 @@ public virtual string Specification
         {
             get
             {
-                return Proxy.git_refspec_string(this.handle);
+                return Proxy.git_refspec_string(handle);
             }
         }
 
@@ -48,7 +47,7 @@ public virtual RefSpecDirection Direction
         {
             get
             {
-                return Proxy.git_refspec_direction(this.handle);
+                return Proxy.git_refspec_direction(handle);
             }
         }
 
@@ -59,7 +58,7 @@ public virtual string Source
         {
             get
             {
-                return Proxy.git_refspec_src(this.handle);
+                return Proxy.git_refspec_src(handle);
             }
         }
 
@@ -70,7 +69,7 @@ public virtual string Destination
         {
             get
             {
-                return Proxy.git_refspec_dst(this.handle);
+                return Proxy.git_refspec_dst(handle);
             }
         }
 
@@ -81,7 +80,7 @@ public virtual bool ForceUpdate
         {
             get
             {
-                return Proxy.git_refspec_force(this.handle);
+                return Proxy.git_refspec_force(handle);
             }
         }
 

LibGit2Sharp/Reference.cs

@@ -32,13 +32,7 @@ private protected Reference(IRepository repo, string canonicalName, string targe
             this.targetIdentifier = targetIdentifier;
         }
 
-        // 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.AsIntPtr(), repo);
-        }
-
-        internal static unsafe T BuildFromPtr<T>(git_reference* handle, Repository repo) where T : Reference
         {
             GitReferenceType type = Proxy.git_reference_type(handle);
             string name = Proxy.git_reference_name(handle);

LibGit2Sharp/ReflogCollection.cs

@@ -63,7 +63,7 @@ public virtual unsafe IEnumerator<ReflogEntry> GetEnumerator()
 
                 for (int i = 0; i < entriesCount; i++)
                 {
-                    git_reflog_entry* handle = Proxy.git_reflog_entry_byindex(reflog, i);
+                    var handle = Proxy.git_reflog_entry_byindex(reflog, i);
                     entries.Add(new ReflogEntry(handle));
                 }
             }

LibGit2Sharp/ReflogEntry.cs

@@ -1,5 +1,4 @@
-using System;
-using System.Runtime.InteropServices;
+using System.Runtime.InteropServices;
 using LibGit2Sharp.Core;
 
 namespace LibGit2Sharp
@@ -25,7 +24,7 @@ protected ReflogEntry()
         /// Initializes a new instance of the <see cref="ReflogEntry"/> class.
         /// </summary>
         /// <param name="entryHandle">a <see cref="SafeHandle"/> to the reflog entry</param>
-        internal unsafe ReflogEntry(git_reflog_entry* entryHandle)
+        internal unsafe ReflogEntry(nint entryHandle)
         {
             _from = Proxy.git_reflog_entry_id_old(entryHandle);
             _to = Proxy.git_reflog_entry_id_new(entryHandle);
@@ -58,7 +57,7 @@ public virtual Signature Committer
         }
 
         /// <summary>
-        /// the message assiocated to this reference update
+        /// the message associated to this reference update
         /// </summary>
         public virtual string Message
         {

LibGit2Sharp/Remote.cs

@@ -135,8 +135,8 @@ internal unsafe string FetchSpecTransformToSource(string reference)
         {
             using (RemoteHandle remoteHandle = Proxy.git_remote_lookup(repository.Handle, Name, true))
             {
-                git_refspec* fetchSpecPtr = Proxy.git_remote_get_refspec(remoteHandle, 0);
-                return Proxy.git_refspec_rtransform(new IntPtr(fetchSpecPtr), reference);
+                var fetchSpecPtr = Proxy.git_remote_get_refspec(remoteHandle, 0);
+                return Proxy.git_refspec_rtransform(fetchSpecPtr, reference);
             }
         }
 

LibGit2Sharp/Repository.cs

@@ -1167,7 +1167,7 @@ public unsafe void RemoveUntrackedFiles()
                                      | CheckoutStrategy.GIT_CHECKOUT_ALLOW_CONFLICTS,
             };
 
-            Proxy.git_checkout_index(Handle, new ObjectHandle(null, false), ref options);
+            Proxy.git_checkout_index(Handle, new ObjectHandle(IntPtr.Zero, false), ref options);
         }
 
         private void CleanupDisposableDependencies()
@@ -1769,13 +1769,14 @@ public string Describe(Commit commit, DescribeOptions options)
         public void RevParse(string revision, out Reference reference, out GitObject obj)
         {
             var handles = Proxy.git_revparse_ext(Handle, revision);
-            if (handles == null)
+
+            if (handles == (null, null))
             {
                 Ensure.GitObjectIsNotNull(null, revision);
             }
 
-            using (var objH = handles.Item1)
-            using (var refH = handles.Item2)
+            using (var objH = handles.obj)
+            using (var refH = handles.reference)
             {
                 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));

LibGit2Sharp/Signature.cs

@@ -15,13 +15,30 @@ public sealed class Signature : IEquatable<Signature>
         private readonly string email;
 
         private static readonly LambdaEqualityHelper<Signature> equalityHelper =
-            new LambdaEqualityHelper<Signature>(x => x.Name, x => x.Email, x => x.When);
+            new(x => x.Name, x => x.Email, x => x.When);
 
-        internal unsafe Signature(git_signature* sig)
+        internal unsafe Signature(SignatureHandle signatureHandle)
         {
-            name = LaxUtf8Marshaler.FromNative(sig->name);
-            email = LaxUtf8Marshaler.FromNative(sig->email);
-            when = DateTimeOffset.FromUnixTimeSeconds(sig->when.time).ToOffset(TimeSpan.FromMinutes(sig->when.offset));
+            var success = false;
+
+            try
+            {
+                signatureHandle.DangerousAddRef(ref success);
+
+                var handle = signatureHandle.DangerousGetHandle();
+                var sig = (git_signature*)handle;
+
+                name = LaxUtf8Marshaler.FromNative(sig->name);
+                email = LaxUtf8Marshaler.FromNative(sig->email);
+                when = DateTimeOffset.FromUnixTimeSeconds(sig->when.time).ToOffset(TimeSpan.FromMinutes(sig->when.offset));
+            }
+            finally
+            {
+                if (success)
+                {
+                    signatureHandle.DangerousRelease();
+                }
+            }
         }
 
         /// <summary>
@@ -146,22 +163,22 @@ public override string ToString()
         }
     }
 
-    internal static class SignatureHelpers
-    {
-        /// <summary>
-        /// Build the handle for the Signature, or return a handle
-        /// to an empty signature.
-        /// </summary>
-        /// <param name="signature"></param>
-        /// <returns></returns>
-        public static unsafe SignatureHandle SafeBuildHandle(this Signature signature)
-        {
-            if (signature == null)
-            {
-                return new SignatureHandle(null, false);
-            }
-
-            return signature.BuildHandle();
-        }
-    }
+    //internal static class SignatureHelpers
+    //{
+    //    /// <summary>
+    //    /// Build the handle for the Signature, or return a handle
+    //    /// to an empty signature.
+    //    /// </summary>
+    //    /// <param name="signature"></param>
+    //    /// <returns></returns>
+    //    public static unsafe SignatureHandle SafeBuildHandle(this Signature signature)
+    //    {
+    //        if (signature == null)
+    //        {
+    //            return new SignatureHandle(null, false);
+    //        }
+
+    //        return signature.BuildHandle();
+    //    }
+    //}
 }