diff --git a/LibGit2Sharp/Core/Handles/ConfigurationSafeHandle.cs b/LibGit2Sharp/Core/Handles/ConfigurationSafeHandle.cs index 2393e3dac..01b2a4c9d 100644 --- a/LibGit2Sharp/Core/Handles/ConfigurationSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/ConfigurationSafeHandle.cs @@ -2,7 +2,7 @@ { internal class ConfigurationSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_config_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/DiffListSafeHandle.cs b/LibGit2Sharp/Core/Handles/DiffListSafeHandle.cs index c78f31317..26d697ac2 100644 --- a/LibGit2Sharp/Core/Handles/DiffListSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/DiffListSafeHandle.cs @@ -2,7 +2,7 @@ namespace LibGit2Sharp.Core.Handles { internal class DiffListSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_diff_list_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/GitObjectSafeHandle.cs b/LibGit2Sharp/Core/Handles/GitObjectSafeHandle.cs index a25d8ecfe..46de2dfe7 100644 --- a/LibGit2Sharp/Core/Handles/GitObjectSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/GitObjectSafeHandle.cs @@ -2,7 +2,7 @@ { internal class GitObjectSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_object_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/IndexSafeHandle.cs b/LibGit2Sharp/Core/Handles/IndexSafeHandle.cs index 53437b33a..d757efb68 100644 --- a/LibGit2Sharp/Core/Handles/IndexSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/IndexSafeHandle.cs @@ -2,7 +2,7 @@ { internal class IndexSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_index_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/NoteSafeHandle.cs b/LibGit2Sharp/Core/Handles/NoteSafeHandle.cs index 513aabdc2..a62c5105a 100644 --- a/LibGit2Sharp/Core/Handles/NoteSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/NoteSafeHandle.cs @@ -2,7 +2,7 @@ { internal class NoteSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_note_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/NullGitObjectSafeHandle.cs b/LibGit2Sharp/Core/Handles/NullGitObjectSafeHandle.cs index af9595043..5c4521193 100644 --- a/LibGit2Sharp/Core/Handles/NullGitObjectSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/NullGitObjectSafeHandle.cs @@ -9,7 +9,7 @@ public NullGitObjectSafeHandle() handle = IntPtr.Zero; } - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { // Nothing to release return true; diff --git a/LibGit2Sharp/Core/Handles/NullIndexSafeHandle.cs b/LibGit2Sharp/Core/Handles/NullIndexSafeHandle.cs index a6bbca84f..dcd4988fb 100644 --- a/LibGit2Sharp/Core/Handles/NullIndexSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/NullIndexSafeHandle.cs @@ -9,7 +9,7 @@ public NullIndexSafeHandle() handle = IntPtr.Zero; } - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { // Nothing to release return true; diff --git a/LibGit2Sharp/Core/Handles/ObjectDatabaseSafeHandle.cs b/LibGit2Sharp/Core/Handles/ObjectDatabaseSafeHandle.cs index 89791e459..e8cfb2b0a 100644 --- a/LibGit2Sharp/Core/Handles/ObjectDatabaseSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/ObjectDatabaseSafeHandle.cs @@ -2,7 +2,7 @@ { internal class ObjectDatabaseSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_odb_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/PushSafeHandle.cs b/LibGit2Sharp/Core/Handles/PushSafeHandle.cs index 9b6ad26c3..ba958d956 100644 --- a/LibGit2Sharp/Core/Handles/PushSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/PushSafeHandle.cs @@ -2,7 +2,7 @@ { internal class PushSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_push_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/ReferenceSafeHandle.cs b/LibGit2Sharp/Core/Handles/ReferenceSafeHandle.cs index d2ebb6147..9ac7d1f7e 100644 --- a/LibGit2Sharp/Core/Handles/ReferenceSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/ReferenceSafeHandle.cs @@ -2,7 +2,7 @@ { internal class ReferenceSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_reference_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/RemoteSafeHandle.cs b/LibGit2Sharp/Core/Handles/RemoteSafeHandle.cs index d7c792deb..d032e34f5 100644 --- a/LibGit2Sharp/Core/Handles/RemoteSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/RemoteSafeHandle.cs @@ -2,7 +2,7 @@ { internal class RemoteSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_remote_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/RepositorySafeHandle.cs b/LibGit2Sharp/Core/Handles/RepositorySafeHandle.cs index 97950cc5d..889a3022c 100644 --- a/LibGit2Sharp/Core/Handles/RepositorySafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/RepositorySafeHandle.cs @@ -2,7 +2,7 @@ { internal class RepositorySafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_repository_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/RevWalkerSafeHandle.cs b/LibGit2Sharp/Core/Handles/RevWalkerSafeHandle.cs index 23449c43c..457bd027c 100644 --- a/LibGit2Sharp/Core/Handles/RevWalkerSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/RevWalkerSafeHandle.cs @@ -2,7 +2,7 @@ { internal class RevWalkerSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_revwalk_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/SafeHandleBase.cs b/LibGit2Sharp/Core/Handles/SafeHandleBase.cs index e51a7562c..284e0d49c 100644 --- a/LibGit2Sharp/Core/Handles/SafeHandleBase.cs +++ b/LibGit2Sharp/Core/Handles/SafeHandleBase.cs @@ -1,7 +1,9 @@ using System; using System.Diagnostics; using System.Globalization; +using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; +using Interlocked = System.Threading.Interlocked; namespace LibGit2Sharp.Core.Handles { @@ -11,9 +13,18 @@ internal abstract class SafeHandleBase : SafeHandle private readonly string trace; #endif + /// + /// This is set to non-zero when has + /// been called for this handle, but + /// has not yet been called. + /// + private int registered; + protected SafeHandleBase() : base(IntPtr.Zero, true) { + NativeMethods.AddHandle(); + registered = 1; #if LEAKS trace = new StackTrace(2, true).ToString(); #endif @@ -35,11 +46,71 @@ protected override void Dispose(bool disposing) } #endif - public override bool IsInvalid + // Prevent the debugger from evaluating this property because it has side effects + [DebuggerBrowsable(DebuggerBrowsableState.Never)] + public override sealed bool IsInvalid + { + get + { + bool invalid = IsInvalidImpl(); + if (invalid && Interlocked.CompareExchange(ref registered, 0, 1) == 1) + { + /* Unregister the handle because we know ReleaseHandle won't be called + * to do it for us. + */ + NativeMethods.RemoveHandle(); + } + + return invalid; + } + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + protected virtual bool IsInvalidImpl() { - get { return (handle == IntPtr.Zero); } + return handle == IntPtr.Zero; } - protected abstract override bool ReleaseHandle(); + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + protected abstract bool ReleaseHandleImpl(); + + protected override sealed bool ReleaseHandle() + { + bool result; + + try + { + result = ReleaseHandleImpl(); + } + finally + { + if (Interlocked.CompareExchange(ref registered, 0, 1) == 1) + { + // if the handle is still registered at this point, we definitely + // want to unregister it + NativeMethods.RemoveHandle(); + } + else + { + /* For this to be called, the following sequence of events must occur: + * + * 1. The handle is created + * 2. The IsInvalid property is evaluated, and the result is false + * 3. The IsInvalid property is evaluated by the runtime to determine if + * finalization is necessary, and the result is now true + * + * This can only happen if the value of `handle` is manipulated in an unexpected + * way (through the Reflection API or by a specially-crafted derived type that + * does not currently exist). The only safe course of action at this point in + * the shutdown process is returning false, which will trigger the + * releaseHandleFailed MDA but have no other impact on the CLR state. + * http://msdn.microsoft.com/en-us/library/85eak4a0.aspx + */ + result = false; + } + } + + return result; + } } } diff --git a/LibGit2Sharp/Core/Handles/SignatureSafeHandle.cs b/LibGit2Sharp/Core/Handles/SignatureSafeHandle.cs index 1db9e1259..51f745ae9 100644 --- a/LibGit2Sharp/Core/Handles/SignatureSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/SignatureSafeHandle.cs @@ -2,7 +2,7 @@ { internal class SignatureSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_signature_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/TreeBuilderSafeHandle.cs b/LibGit2Sharp/Core/Handles/TreeBuilderSafeHandle.cs index 5f477f6cc..1fd17a292 100644 --- a/LibGit2Sharp/Core/Handles/TreeBuilderSafeHandle.cs +++ b/LibGit2Sharp/Core/Handles/TreeBuilderSafeHandle.cs @@ -2,7 +2,7 @@ namespace LibGit2Sharp.Core.Handles { internal class TreeBuilderSafeHandle : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_treebuilder_free(handle); return true; diff --git a/LibGit2Sharp/Core/Handles/TreeEntrySafeHandle_Owned.cs b/LibGit2Sharp/Core/Handles/TreeEntrySafeHandle_Owned.cs index 8ba4c9f20..f2d63e13f 100644 --- a/LibGit2Sharp/Core/Handles/TreeEntrySafeHandle_Owned.cs +++ b/LibGit2Sharp/Core/Handles/TreeEntrySafeHandle_Owned.cs @@ -2,7 +2,7 @@ { internal class TreeEntrySafeHandle_Owned : SafeHandleBase { - protected override bool ReleaseHandle() + protected override bool ReleaseHandleImpl() { Proxy.git_tree_entry_free(handle); return true; diff --git a/LibGit2Sharp/Core/NativeMethods.cs b/LibGit2Sharp/Core/NativeMethods.cs index a8650829e..17e244e0c 100644 --- a/LibGit2Sharp/Core/NativeMethods.cs +++ b/LibGit2Sharp/Core/NativeMethods.cs @@ -1,10 +1,12 @@ using System; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; +using System.Threading; using LibGit2Sharp.Core.Handles; // ReSharper disable InconsistentNaming @@ -15,6 +17,7 @@ internal static class NativeMethods public const uint GIT_PATH_MAX = 4096; private const string libgit2 = "git2"; private static readonly LibraryLifetimeObject lifetimeObject; + private static int handlesCount = 0; /// /// Internal hack to ensure that the call to git_threads_shutdown is called after all handle finalizers @@ -26,8 +29,32 @@ private sealed class LibraryLifetimeObject : CriticalFinalizerObject // Ensure mono can JIT the .cctor and adjust the PATH before trying to load the native library. // See https://github.com/libgit2/libgit2sharp/pull/190 [MethodImpl(MethodImplOptions.NoInlining)] - public LibraryLifetimeObject() { Ensure.ZeroResult(git_threads_init()); } - ~LibraryLifetimeObject() { git_threads_shutdown(); } + public LibraryLifetimeObject() + { + Ensure.ZeroResult(git_threads_init()); + AddHandle(); + } + + ~LibraryLifetimeObject() + { + RemoveHandle(); + } + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + internal static void AddHandle() + { + Interlocked.Increment(ref handlesCount); + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + internal static void RemoveHandle() + { + int count = Interlocked.Decrement(ref handlesCount); + if (count == 0) + { + git_threads_shutdown(); + } } static NativeMethods()