From 57662acf0916982ae0d863650a183481e6b664e9 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 18 Apr 2017 14:33:24 +0100 Subject: [PATCH 1/4] NativeMethods: drop unnecessary libgit2 refcount Remove the refcounting of libgit2 usage leftover from when `SafeHandleBase` ensured that all handles were finished before calling `git_libgit2_shutdown`. libgit2 itself does this refcounting now, we no longer need to. We only need to call `git_libgit2_init` before the first call to libgit2 and `git_libgit2_shutdown` at the end. --- LibGit2Sharp/Core/NativeMethods.cs | 52 +++++++----------------------- 1 file changed, 11 insertions(+), 41 deletions(-) diff --git a/LibGit2Sharp/Core/NativeMethods.cs b/LibGit2Sharp/Core/NativeMethods.cs index f2ea4f68a..0639d32d2 100644 --- a/LibGit2Sharp/Core/NativeMethods.cs +++ b/LibGit2Sharp/Core/NativeMethods.cs @@ -17,58 +17,28 @@ internal static partial class NativeMethods { public const uint GIT_PATH_MAX = 4096; private const string libgit2 = NativeDllName.Name; - // This is here to keep the pointer alive -#pragma warning disable 0414 + + // An object tied to the lifecycle of the NativeMethods static class. + // This will handle initialization and shutdown of the underlying + // native library. + #pragma warning disable 0414 private static readonly LibraryLifetimeObject lifetimeObject; -#pragma warning restore 0414 - private static int handlesCount; - - /// - /// Internal hack to ensure that the call to git_threads_shutdown is called after all handle finalizers - /// have run to completion ensuring that no dangling git-related finalizer runs after git_threads_shutdown. - /// There should never be more than one instance of this object per AppDomain. - /// - private sealed class LibraryLifetimeObject -#if DESKTOP - : CriticalFinalizerObject -#endif + #pragma warning restore 0414 + + 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() { - int res = git_libgit2_init(); - Ensure.Int32Result(res); - if (res == 1) + // Configure the OpenSSL locking on the true initialization + // of the library. + if (git_libgit2_init() == 1) { - // Ignore the error that this propagates. Call it in case openssl is being used. git_openssl_set_locking(); } - AddHandle(); } ~LibraryLifetimeObject() - { - RemoveHandle(); - } - } - -#if DESKTOP - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] -#endif - internal static void AddHandle() - { - Interlocked.Increment(ref handlesCount); - } - -#if DESKTOP - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] -#endif - internal static void RemoveHandle() - { - int count = Interlocked.Decrement(ref handlesCount); - if (count == 0) { git_libgit2_shutdown(); } From 2d0526516a05f3c674988e8410e4e74c0c12ba72 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 18 Apr 2017 14:59:29 +0100 Subject: [PATCH 2/4] NativeMethods: setup shutdown handler only when init succeeds Only set up the object with the `git_libgit2_shutdown` finalizer when `git_libgit2_init` has succeeded. This ensures that setting up the finalizer is the last thing that we do in the static constructor for `NativeMethods`, meaning that any exception trying to p/invoke `git_libgit2_init` remains catch-able. --- LibGit2Sharp/Core/NativeMethods.cs | 47 ++++++++++++++++-------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/LibGit2Sharp/Core/NativeMethods.cs b/LibGit2Sharp/Core/NativeMethods.cs index 0639d32d2..32d7587ff 100644 --- a/LibGit2Sharp/Core/NativeMethods.cs +++ b/LibGit2Sharp/Core/NativeMethods.cs @@ -22,28 +22,9 @@ internal static partial class NativeMethods // This will handle initialization and shutdown of the underlying // native library. #pragma warning disable 0414 - private static readonly LibraryLifetimeObject lifetimeObject; + private static readonly NativeShutdownObject shutdownObject; #pragma warning restore 0414 - private sealed class LibraryLifetimeObject : CriticalFinalizerObject - { - [MethodImpl(MethodImplOptions.NoInlining)] - public LibraryLifetimeObject() - { - // Configure the OpenSSL locking on the true initialization - // of the library. - if (git_libgit2_init() == 1) - { - git_openssl_set_locking(); - } - } - - ~LibraryLifetimeObject() - { - git_libgit2_shutdown(); - } - } - static NativeMethods() { if (Platform.OperatingSystem == OperatingSystemType.Windows) @@ -57,8 +38,30 @@ static NativeMethods() String.Format(CultureInfo.InvariantCulture, "{0}{1}{2}", path, Path.PathSeparator, Environment.GetEnvironmentVariable(pathEnvVariable))); } - // See LibraryLifetimeObject description. - lifetimeObject = new LibraryLifetimeObject(); + LoadNativeLibrary(); + shutdownObject = new NativeShutdownObject(); + } + + // Avoid inlining this method because otherwise mono's JITter may try + // to load the library _before_ we've configured the path. + [MethodImpl(MethodImplOptions.NoInlining)] + private static void LoadNativeLibrary() + { + // Configure the OpenSSL locking on the true initialization + // of the library. + if (git_libgit2_init() == 1) + { + git_openssl_set_locking(); + } + } + + // Shutdown the native library in a finalizer. + private sealed class NativeShutdownObject : CriticalFinalizerObject + { + ~NativeShutdownObject() + { + git_libgit2_shutdown(); + } } [DllImport(libgit2)] From b21e2f9e0ab6d6e235f28094e51ead8047103bf1 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 24 Jun 2017 15:48:20 +0100 Subject: [PATCH 3/4] NativeMethods: drop the `CriticalFinalizerObject` .NET Core no longer supports CriticalFinalizerObject. We now get the strictest finalization guarantees simply inheriting Object. See https://github.com/dotnet/corefx/issues/1345 --- LibGit2Sharp/Core/NativeMethods.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LibGit2Sharp/Core/NativeMethods.cs b/LibGit2Sharp/Core/NativeMethods.cs index 32d7587ff..3156942f3 100644 --- a/LibGit2Sharp/Core/NativeMethods.cs +++ b/LibGit2Sharp/Core/NativeMethods.cs @@ -56,7 +56,7 @@ private static void LoadNativeLibrary() } // Shutdown the native library in a finalizer. - private sealed class NativeShutdownObject : CriticalFinalizerObject + private sealed class NativeShutdownObject { ~NativeShutdownObject() { From 9ed8a4781503b7b7de07fa711bea8ae0d7fa30f7 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 24 Jun 2017 16:00:02 +0100 Subject: [PATCH 4/4] NativeMethods: update change log --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 790f7a84f..4dfb74cbf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,9 @@ ### Fixes + - The exception thrown when the native library cannot be loaded is now + able to be caught and will no longer crash the process. + ## v0.24 - ([diff](https://github.com/libgit2/libgit2sharp/compare/v0.23..v0.24)) This is the last release before a moving to .NET Core compatible library.