Merge pull request #1563 from tmat/NativeLoad

 Avoid reading and writing global state when loading native library

Author: ethomson

LibGit2Sharp.Tests/GlobalSettingsFixture.cs

@@ -1,4 +1,7 @@
-using System.Text.RegularExpressions;
+using System;
+using System.IO;
+using System.Text.RegularExpressions;
+using LibGit2Sharp.Core;
 using LibGit2Sharp.Tests.TestHelpers;
 using Xunit;
 
@@ -49,5 +52,34 @@ public void TryingToResetNativeLibraryPathAfterLoadedThrows()
 
             Assert.Throws<LibGit2SharpException>(() => { GlobalSettings.NativeLibraryPath = "C:/Foo"; });
         }
+
+        [SkippableTheory]
+        [InlineData("x86")]
+        [InlineData("x64")]
+        public void LoadFromSpecifiedPath(string architecture)
+        {
+            Skip.IfNot(Platform.IsRunningOnNetFramework(), ".NET Framework only test.");
+
+            var nativeDllFileName = NativeDllName.Name + ".dll";
+            var testDir = Path.GetDirectoryName(typeof(GlobalSettingsFixture).Assembly.Location);
+            var testAppExe = Path.Combine(testDir, $"NativeLibraryLoadTestApp.{architecture}.exe");
+            var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+            var platformDir = Path.Combine(tempDir, "plat");
+
+            try
+            {
+                Directory.CreateDirectory(Path.Combine(platformDir, architecture));
+                File.Copy(Path.Combine(GlobalSettings.NativeLibraryPath, architecture, nativeDllFileName), Path.Combine(platformDir, architecture, nativeDllFileName));
+
+                var (output, exitCode) = ProcessHelper.RunProcess(testAppExe, arguments: $@"{NativeDllName.Name} ""{platformDir}""", workingDirectory: tempDir);
+
+                Assert.Empty(output);
+                Assert.Equal(0, exitCode);
+            }
+            finally
+            {
+                DirectoryHelper.DeleteDirectory(tempDir);
+            }
+        }
     }
 }

LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj

@@ -7,6 +7,8 @@
 
   <ItemGroup>
     <ProjectReference Include="..\LibGit2Sharp\LibGit2Sharp.csproj" />
+    <ProjectReference Include="..\NativeLibraryLoadTestApp\x86\NativeLibraryLoadTestApp.x86.csproj" Condition="'$(TargetFramework)' == 'net461'" ReferenceOutputAssembly="false" OutputItemType="TestAppExe" />
+    <ProjectReference Include="..\NativeLibraryLoadTestApp\x64\NativeLibraryLoadTestApp.x64.csproj" Condition="'$(TargetFramework)' == 'net461'" ReferenceOutputAssembly="false" OutputItemType="TestAppExe" />
   </ItemGroup>
 
   <ItemGroup>
@@ -24,4 +26,18 @@
     <Compile Remove="desktop\**" Condition=" '$(TargetFramework)' != 'net461' " />
   </ItemGroup>
 
+  <Target Name="CopyTestAppExes" AfterTargets="ResolveProjectReferences">
+    <ItemGroup>
+      <_TestAppFile Include="@(TestAppExe->'%(RootDir)%(Directory)%(Filename).exe')" />
+      <_TestAppFile Include="@(TestAppExe->'%(RootDir)%(Directory)%(Filename).exe.config')" />
+      <_TestAppFile Include="@(TestAppExe->'%(RootDir)%(Directory)%(Filename).pdb')" />
+    </ItemGroup>
+
+    <ItemGroup>
+      <Content Include="@(_TestAppFile)" CopyToOutputDirectory="PreserveNewest" Visible="false" />
+    </ItemGroup>
+  </Target>
+
+  <Import Project="..\Targets\GenerateNativeDllName.targets" />
+
 </Project>

LibGit2Sharp.Tests/TestHelpers/ProcessHelper.cs

@@ -0,0 +1,36 @@
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Text;
+
+namespace LibGit2Sharp.Tests
+{
+    public static class ProcessHelper
+    {
+        public static (string, int) RunProcess(string fileName, string arguments, string workingDirectory = null)
+        {
+            var process = new Process
+            {
+                StartInfo = new ProcessStartInfo(fileName, arguments)
+                {
+                    RedirectStandardError = true,
+                    RedirectStandardOutput = true,
+                    CreateNoWindow = true,
+                    UseShellExecute = false,
+                    WorkingDirectory = workingDirectory ?? string.Empty
+                }
+            };
+
+            var output = new StringBuilder();
+
+            process.OutputDataReceived += (_, e) => output.AppendLine(e.Data);
+            process.ErrorDataReceived += (_, e) => output.AppendLine(e.Data);
+
+            process.Start();
+
+            process.WaitForExit();
+
+            return (output.ToString(), process.ExitCode);
+        }
+    }
+}

LibGit2Sharp.sln

@@ -9,12 +9,18 @@ EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{0CA739FD-DA4D-4F64-9834-DA14A3ECD04B}"
 	ProjectSection(SolutionItems) = preProject
 		.gitignore = .gitignore
+		Targets\CodeGenerator.targets = Targets\CodeGenerator.targets
 		Directory.Build.props = Directory.Build.props
 		Directory.Build.targets = Directory.Build.targets
+		Targets\GenerateNativeDllName.targets = Targets\GenerateNativeDllName.targets
 		nuget.config = nuget.config
 		version.json = version.json
 	EndProjectSection
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NativeLibraryLoadTestApp.x86", "NativeLibraryLoadTestApp\x86\NativeLibraryLoadTestApp.x86.csproj", "{86453D2C-4953-4DF4-B12A-ADE579608BAA}"
+EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NativeLibraryLoadTestApp.x64", "NativeLibraryLoadTestApp\x64\NativeLibraryLoadTestApp.x64.csproj", "{5C55175D-6A1F-4C51-B791-BF7DD00124EE}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -29,6 +35,14 @@ Global
 		{286E63EB-04DD-4ADE-88D6-041B57800761}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{286E63EB-04DD-4ADE-88D6-041B57800761}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{286E63EB-04DD-4ADE-88D6-041B57800761}.Release|Any CPU.Build.0 = Release|Any CPU
+		{86453D2C-4953-4DF4-B12A-ADE579608BAA}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{86453D2C-4953-4DF4-B12A-ADE579608BAA}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{86453D2C-4953-4DF4-B12A-ADE579608BAA}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{86453D2C-4953-4DF4-B12A-ADE579608BAA}.Release|Any CPU.Build.0 = Release|Any CPU
+		{5C55175D-6A1F-4C51-B791-BF7DD00124EE}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{5C55175D-6A1F-4C51-B791-BF7DD00124EE}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{5C55175D-6A1F-4C51-B791-BF7DD00124EE}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{5C55175D-6A1F-4C51-B791-BF7DD00124EE}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE

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,68 @@ 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() || Platform.IsRunningOnNetCore())
             {
-                string nativeLibraryPath = GlobalSettings.GetAndLockNativeLibraryPath();
-
-                string path = Path.Combine(nativeLibraryPath, Platform.ProcessorArchitecture);
-
-                const string pathEnvVariable = "PATH";
-                Environment.SetEnvironmentVariable(pathEnvVariable,
-                    String.Format(CultureInfo.InvariantCulture, "{0}{1}{2}", path, Path.PathSeparator, Environment.GetEnvironmentVariable(pathEnvVariable)));
+                string nativeLibraryDir = GlobalSettings.GetAndLockNativeLibraryPath();
+                if (nativeLibraryDir != null)
+                {
+                    string nativeLibraryPath = Path.Combine(nativeLibraryDir, libgit2 + Platform.GetNativeLibraryExtension());
+
+                    // 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.
+                    if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+                    {
+                        LoadWindowsLibrary(nativeLibraryPath);
+                    }
+                    else
+                    {
+                        LoadUnixLibrary(nativeLibraryPath, RTLD_NOW);
+                    }
+                }
             }
 
-            LoadNativeLibrary();
-            shutdownObject = new NativeShutdownObject();
+            InitializeNativeLibrary();
         }
 
+        public const int RTLD_NOW = 0x002;
+
+        [DllImport("libdl", EntryPoint = "dlopen")]
+        private static extern IntPtr LoadUnixLibrary(string path, int flags);
+
+        [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

@@ -33,8 +33,43 @@ public static OperatingSystemType OperatingSystem
                     return OperatingSystemType.MacOSX;
                 }
 
-                throw new InvalidOperationException();
+                throw new PlatformNotSupportedException();
             }
         }
+
+        public static string GetNativeLibraryExtension()
+        {
+            switch (OperatingSystem)
+            {
+                case OperatingSystemType.MacOSX:
+                    return ".dylib";
+
+                case OperatingSystemType.Unix:
+                    return ".so";
+
+                case OperatingSystemType.Windows:
+                    return ".dll";
+            }
+
+            throw new PlatformNotSupportedException();
+        }
+
+        /// <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();
+
+        /// <summary>
+        /// Returns true if the runtime is .NET Core.
+        /// </summary>
+        public static bool IsRunningOnNetCore()
+            => typeof(object).Assembly.GetName().Name != "mscorlib";
     }
 }