Skip to content

Use SafeHandles #2127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 21, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 23 additions & 64 deletions LibGit2Sharp/Core/Handles/Libgit2Object.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}

250 changes: 150 additions & 100 deletions LibGit2Sharp/Core/Handles/Objects.cs

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions LibGit2Sharp/Core/Handles/Objects.tt
Original file line number Diff line number Diff line change
@@ -73,7 +73,7 @@ for (var i = 0; i < cNames.Length; i++)
internal unsafe class <#= csNames[i] #> : Libgit2Object
{
internal <#= csNames[i] #>(<#= cNames[i] #> *ptr, bool owned)
: base((void *) ptr, owned)
: base(ptr, owned)
{
}

@@ -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] #>*)AsIntPtr());

return true;
}

public static implicit operator <#= cNames[i] #>*(<#= csNames[i] #> handle)
{
return (<#= cNames[i] #>*) handle.Handle;
return (<#= cNames[i] #>*)handle.AsIntPtr();
}
}

4 changes: 2 additions & 2 deletions LibGit2Sharp/Network.cs
Original file line number Diff line number Diff line change
@@ -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;
}
2 changes: 1 addition & 1 deletion LibGit2Sharp/Reference.cs
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions LibGit2Sharp/Repository.cs
Original file line number Diff line number Diff line change
@@ -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));
}
}