Avoid reading and writing global state when loading native library

Author: tmat

LibGit2Sharp/Core/NativeMethods.cs

@@ -1,10 +1,15 @@
 using System;
-using System.Globalization;
 using System.IO;
 using System.Runtime.CompilerServices;
+using System.Runtime.ConstrainedExecution;
 using System.Runtime.InteropServices;
 using LibGit2Sharp.Core.Handles;
 
+// Restrict the set of directories where the native library is loaded from to safe directories.
+[assembly: DefaultDllImportSearchPaths(DllImportSearchPath.AssemblyDirectory | DllImportSearchPath.ApplicationDirectory | DllImportSearchPath.SafeDirectories)]
+
+#pragma warning disable IDE1006 // Naming Styles
+
 // ReSharper disable InconsistentNaming
 namespace LibGit2Sharp.Core
 {
@@ -17,41 +22,52 @@ internal static class NativeMethods
         // This will handle initialization and shutdown of the underlying
         // native library.
 #pragma warning disable 0414
-        private static readonly NativeShutdownObject shutdownObject;
-        #pragma warning restore 0414
+        private static NativeShutdownObject shutdownObject;
+#pragma warning restore 0414
 
         static NativeMethods()
         {
-            if (Platform.OperatingSystem == OperatingSystemType.Windows)
+            if (Platform.IsRunningOnNetFramework())
             {
-                string nativeLibraryPath = GlobalSettings.GetAndLockNativeLibraryPath();
-
-                string path = Path.Combine(nativeLibraryPath, Platform.ProcessorArchitecture);
+                string dllPath = Path.Combine(GlobalSettings.GetAndLockNativeLibraryPath(), libgit2 + ".dll");
 
-                const string pathEnvVariable = "PATH";
-                Environment.SetEnvironmentVariable(pathEnvVariable,
-                    String.Format(CultureInfo.InvariantCulture, "{0}{1}{2}", path, Path.PathSeparator, Environment.GetEnvironmentVariable(pathEnvVariable)));
+                // Try to load the .dll from the path explicitly.
+                // If this call succeeds further DllImports will find the library loaded and not attempt to load it again.
+                // If it fails the next DllImport will load the library from safe directories.
+                LoadWindowsLibrary(dllPath);
             }
 
-            LoadNativeLibrary();
-            shutdownObject = new NativeShutdownObject();
+            InitializeNativeLibrary();
         }
 
+        [DllImport("kernel32", EntryPoint = "LoadLibrary")]
+        private static extern IntPtr LoadWindowsLibrary(string path);
+
         // 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()
+        private static void InitializeNativeLibrary()
         {
-            // Configure the OpenSSL locking on the true initialization
-            // of the library.
-            if (git_libgit2_init() == 1)
+            int initCounter;
+            try
+            {
+            }
+            finally // avoid thread aborts
+            {
+                // Initialization can be called multiple times as long as there is a corresponding shutdown to each initialization.
+                initCounter = git_libgit2_init();
+                shutdownObject = new NativeShutdownObject();
+            }
+
+            // Configure the OpenSSL locking on the first initialization of the library in the current process.
+            if (initCounter == 1)
             {
                 git_openssl_set_locking();
             }
         }
 
         // Shutdown the native library in a finalizer.
-        private sealed class NativeShutdownObject
+        private sealed class NativeShutdownObject : CriticalFinalizerObject
         {
             ~NativeShutdownObject()
             {

LibGit2Sharp/Core/Platform.cs

@@ -36,5 +36,17 @@ public static OperatingSystemType OperatingSystem
                 throw new InvalidOperationException();
             }
         }
+
+        /// <summary>
+        /// Returns true if the runtime is Mono.
+        /// </summary>
+        public static bool IsRunningOnMono()
+            => Type.GetType("Mono.Runtime") != null;
+
+        /// <summary>
+        /// Returns true if the runtime is .NET Framework.
+        /// </summary>
+        public static bool IsRunningOnNetFramework()
+            => typeof(object).Assembly.GetName().Name == "mscorlib" && !IsRunningOnMono();
     }
 }

LibGit2Sharp/GlobalSettings.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Reflection;
+using System.Runtime.InteropServices;
 using LibGit2Sharp.Core;
 
 namespace LibGit2Sharp
@@ -13,6 +14,7 @@ public static class GlobalSettings
     {
         private static readonly Lazy<Version> version = new Lazy<Version>(Version.Build);
         private static readonly Dictionary<Filter, FilterRegistration> registeredFilters;
+        private static readonly bool nativeLibraryPathAllowed;
 
         private static LogConfiguration logConfiguration = LogConfiguration.None;
 
@@ -21,7 +23,9 @@ public static class GlobalSettings
 
         static GlobalSettings()
         {
-            if (Platform.OperatingSystem == OperatingSystemType.Windows)
+            nativeLibraryPathAllowed = Platform.IsRunningOnNetFramework();
+
+            if (nativeLibraryPathAllowed)
             {
                 /* Assembly.CodeBase is not actually a correctly formatted
                  * URI.  It's merely prefixed with `file:///` and has its
@@ -148,23 +152,24 @@ public static LogConfiguration LogConfiguration
         }
 
         /// <summary>
-        /// Sets a hint path for searching for native binaries: when
-        /// specified, native binaries will first be searched in a
-        /// subdirectory of the given path corresponding to the operating
-        /// system and architecture (eg, "x86" or "x64") before falling
-        /// back to the default path ("lib\win32\x86" or "lib\win32\x64"
-        /// next to the application).
+        /// Sets a path for loading native binaries on .NET Framework.
+        /// When specified, native .dll will first be searched in a
+        /// subdirectory of the given path corresponding to the
+        /// architecture ("x86" or "x64") before falling
+        /// back to searching the default load directories
+        /// (<see cref="DllImportSearchPath.AssemblyDirectory"/>,
+        /// <see cref="DllImportSearchPath.ApplicationDirectory"/> and
+        /// <see cref="DllImportSearchPath.SafeDirectories"/>).
         /// <para>
         /// This must be set before any other calls to the library,
-        /// and is not available on Unix platforms: see your dynamic
-        /// library loader's documentation for details.
+        /// and is not available on other platforms than .NET Framework.
         /// </para>
         /// </summary>
         public static string NativeLibraryPath
         {
             get
             {
-                if (Platform.OperatingSystem != OperatingSystemType.Windows)
+                if (!nativeLibraryPathAllowed)
                 {
                     throw new LibGit2SharpException("Querying the native hint path is only supported on Windows platforms");
                 }
@@ -174,7 +179,7 @@ public static string NativeLibraryPath
 
             set
             {
-                if (Platform.OperatingSystem != OperatingSystemType.Windows)
+                if (!nativeLibraryPathAllowed)
                 {
                     throw new LibGit2SharpException("Setting the native hint path is only supported on Windows platforms");
                 }
@@ -184,14 +189,21 @@ public static string NativeLibraryPath
                     throw new LibGit2SharpException("You cannot set the native library path after it has been loaded");
                 }
 
-                nativeLibraryPath = value;
+                try
+                {
+                    nativeLibraryPath = Path.GetFullPath(value);
+                }
+                catch (Exception e)
+                {
+                    throw new LibGit2SharpException(e.Message);
+                }
             }
         }
 
         internal static string GetAndLockNativeLibraryPath()
         {
             nativeLibraryPathLocked = true;
-            return nativeLibraryPath;
+            return Path.Combine(nativeLibraryPath, Platform.ProcessorArchitecture);
         }
 
         /// <summary>