Repository: ignite
Updated Branches:
  refs/heads/ignite-3478 1508e665d -> ac475bc18


IGNITE-5730 .NET: Fix ignite.jni.dll temp dir race

This closes #2755


Project: http://git-wip-us.apache.org/repos/asf/ignite/repo
Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/7f823401
Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/7f823401
Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/7f823401

Branch: refs/heads/ignite-3478
Commit: 7f82340189bebac7bfd5f935eb3531035a6f9acb
Parents: cccf113
Author: Pavel Tupitsyn <[email protected]>
Authored: Wed Sep 27 12:20:24 2017 +0300
Committer: Pavel Tupitsyn <[email protected]>
Committed: Wed Sep 27 12:20:24 2017 +0300

----------------------------------------------------------------------
 .../ConsoleRedirectTest.cs                      | 10 ++-
 .../Apache.Ignite.Core/Impl/IgniteUtils.cs      | 66 +++++++++++++-------
 .../Apache.Ignite.Core/Impl/NativeMethods.cs    | 11 +++-
 .../Impl/Unmanaged/UnmanagedUtils.cs            | 51 +++++++++++++--
 4 files changed, 110 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ignite/blob/7f823401/modules/platforms/dotnet/Apache.Ignite.Core.Tests/ConsoleRedirectTest.cs
----------------------------------------------------------------------
diff --git 
a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/ConsoleRedirectTest.cs 
b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/ConsoleRedirectTest.cs
index 3ab5ed3..4ab27ef 100644
--- a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/ConsoleRedirectTest.cs
+++ b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/ConsoleRedirectTest.cs
@@ -149,6 +149,8 @@ namespace Apache.Ignite.Core.Tests
                     typeof(IgniteRunner).Assembly.FullName, 
typeof(IgniteRunner).FullName);
 
                 runner.Run();
+
+                Assert.AreEqual("newDomainGrid", runner.IgniteName);
             }
             finally
             {
@@ -160,19 +162,25 @@ namespace Apache.Ignite.Core.Tests
         private interface IIgniteRunner
         {
             void Run();
+
+            string IgniteName { get; }
         }
 
         private class IgniteRunner : MarshalByRefObject, IIgniteRunner
         {
+            private static IIgnite _ignite;
+
             public void Run()
             {
-                Ignition.Start(new 
IgniteConfiguration(TestUtils.GetTestConfiguration())
+                _ignite = Ignition.Start(new 
IgniteConfiguration(TestUtils.GetTestConfiguration())
                 {
                     IgniteInstanceName = "newDomainGrid"
                 });
 
                 // Will be stopped automatically on domain unload.
             }
+
+            public string IgniteName { get { return _ignite.Name; } }
         }
     }
 }

http://git-wip-us.apache.org/repos/asf/ignite/blob/7f823401/modules/platforms/dotnet/Apache.Ignite.Core/Impl/IgniteUtils.cs
----------------------------------------------------------------------
diff --git a/modules/platforms/dotnet/Apache.Ignite.Core/Impl/IgniteUtils.cs 
b/modules/platforms/dotnet/Apache.Ignite.Core/Impl/IgniteUtils.cs
index f3bdd2b..7e1e8d7 100644
--- a/modules/platforms/dotnet/Apache.Ignite.Core/Impl/IgniteUtils.cs
+++ b/modules/platforms/dotnet/Apache.Ignite.Core/Impl/IgniteUtils.cs
@@ -20,6 +20,7 @@ namespace Apache.Ignite.Core.Impl
     using System;
     using System.Collections.Generic;
     using System.ComponentModel;
+    using System.Diagnostics;
     using System.Diagnostics.CodeAnalysis;
     using System.Globalization;
     using System.IO;
@@ -72,7 +73,7 @@ namespace Apache.Ignite.Core.Impl
         internal const string FileIgniteJniDll = "ignite.jni.dll";
         
         /** Prefix for temp directory names. */
-        private const string DirIgniteTmp = "Ignite_";
+        private static readonly string DirIgniteTmp = 
Path.Combine(Path.GetTempPath(), "Ignite_");
         
         /** Loaded. */
         private static bool _loaded;        
@@ -82,16 +83,6 @@ namespace Apache.Ignite.Core.Impl
         private static Random _rnd;
 
         /// <summary>
-        /// Initializes the <see cref="IgniteUtils"/> class.
-        /// </summary>
-        [SuppressMessage("Microsoft.Performance", 
"CA1810:InitializeReferenceTypeStaticFieldsInline",
-            Justification = "Readability.")]
-        static IgniteUtils()
-        {
-            TryCleanTempDirectories();
-        }
-
-        /// <summary>
         /// Gets thread local random.
         /// </summary>
         /// <value>Thread local random.</value>
@@ -386,15 +377,18 @@ namespace Apache.Ignite.Core.Impl
         }
 
         /// <summary>
-        /// Tries to clean temporary directories created with <see 
cref="GetTempDirectoryName"/>.
+        /// Creates a uniquely named, empty temporary directory on disk and 
returns the full path of that directory.
         /// </summary>
-        private static void TryCleanTempDirectories()
+        /// <returns>The full path of the temporary directory.</returns>
+        internal static string GetTempDirectoryName()
         {
-            foreach (var dir in Directory.GetDirectories(Path.GetTempPath(), 
DirIgniteTmp + "*"))
+            while (true)
             {
+                var dir = DirIgniteTmp + Path.GetRandomFileName();
+
                 try
                 {
-                    Directory.Delete(dir, true);
+                    return Directory.CreateDirectory(dir).FullName;
                 }
                 catch (IOException)
                 {
@@ -408,18 +402,48 @@ namespace Apache.Ignite.Core.Impl
         }
 
         /// <summary>
-        /// Creates a uniquely named, empty temporary directory on disk and 
returns the full path of that directory.
+        /// Unloads the jni DLL and removes temporary directory.
         /// </summary>
-        /// <returns>The full path of the temporary directory.</returns>
-        internal static string GetTempDirectoryName()
+        internal static void UnloadJniDllAndRemoveTempDirectory()
         {
-            while (true)
+            // Unload unmanaged dlls and remove temp folders.
+            // Multiple AppDomains could load multiple instances of the dll, 
so iterate over all modules.
+            foreach (ProcessModule mod in Process.GetCurrentProcess().Modules)
             {
-                var dir = Path.Combine(Path.GetTempPath(), DirIgniteTmp + 
Path.GetRandomFileName());
+                if (mod.ModuleName != FileIgniteJniDll)
+                {
+                    continue;
+                }
 
+                UnloadJniDllAndRemoveTempDirectory(mod.BaseAddress, 
mod.FileName);
+            }
+        }
+
+        /// <summary>
+        /// Unloads the jni DLL and removes temporary directory.
+        /// </summary>
+        internal static void UnloadJniDllAndRemoveTempDirectory(IntPtr 
address, string fileName)
+        {
+            var dir = Path.GetDirectoryName(fileName);
+
+            if (dir == null || !dir.StartsWith(DirIgniteTmp))
+            {
+                return;
+            }
+
+            while (NativeMethods.FreeLibrary(address))
+            {
+                // No-op.
+                // FreeLibrary needs to be called multiple times, because each 
DllImport increases reference count.
+            }
+
+            // Retry 3 times: FreeLibrary might have a delay.
+            for (var i = 0; i < 3; i++)
+            {
                 try
                 {
-                    return Directory.CreateDirectory(dir).FullName;
+                    Directory.Delete(dir, true);
+                    break;
                 }
                 catch (IOException)
                 {

http://git-wip-us.apache.org/repos/asf/ignite/blob/7f823401/modules/platforms/dotnet/Apache.Ignite.Core/Impl/NativeMethods.cs
----------------------------------------------------------------------
diff --git a/modules/platforms/dotnet/Apache.Ignite.Core/Impl/NativeMethods.cs 
b/modules/platforms/dotnet/Apache.Ignite.Core/Impl/NativeMethods.cs
index 0004772..859a04e 100644
--- a/modules/platforms/dotnet/Apache.Ignite.Core/Impl/NativeMethods.cs
+++ b/modules/platforms/dotnet/Apache.Ignite.Core/Impl/NativeMethods.cs
@@ -38,15 +38,22 @@ namespace Apache.Ignite.Core.Impl
         public const int ERROR_MOD_NOT_FOUND = 126;
 
         /// <summary>
-        /// Load DLL with WinAPI.
+        /// Loads unmanaged DLL with WinAPI.
         /// </summary>
         /// <param name="path">Path to dll.</param>
-        /// <returns></returns>
+        /// <returns>Pointer to the loaded library.</returns>
         [DllImport("kernel32.dll", SetLastError = true, CharSet = 
CharSet.Ansi, BestFitMapping = false, 
             ThrowOnUnmappableChar = true)]
         internal static extern IntPtr LoadLibrary(string path);
 
         /// <summary>
+        /// Unloads the unmanaged DLL loaded with <see cref="LoadLibrary"/>.
+        /// </summary>
+        /// <param name="hModule">Pointer from <see 
cref="LoadLibrary"/>.</param>
+        [DllImport("kernel32", SetLastError = true)]
+        internal static extern bool FreeLibrary(IntPtr hModule);
+
+        /// <summary>
         /// Gets the total physical memory.
         /// </summary>
         internal static ulong GetTotalPhysicalMemory()

http://git-wip-us.apache.org/repos/asf/ignite/blob/7f823401/modules/platforms/dotnet/Apache.Ignite.Core/Impl/Unmanaged/UnmanagedUtils.cs
----------------------------------------------------------------------
diff --git 
a/modules/platforms/dotnet/Apache.Ignite.Core/Impl/Unmanaged/UnmanagedUtils.cs 
b/modules/platforms/dotnet/Apache.Ignite.Core/Impl/Unmanaged/UnmanagedUtils.cs
index b6e6582..e783586 100644
--- 
a/modules/platforms/dotnet/Apache.Ignite.Core/Impl/Unmanaged/UnmanagedUtils.cs
+++ 
b/modules/platforms/dotnet/Apache.Ignite.Core/Impl/Unmanaged/UnmanagedUtils.cs
@@ -29,6 +29,15 @@ namespace Apache.Ignite.Core.Impl.Unmanaged
     /// </summary>
     internal static unsafe class UnmanagedUtils
     {
+        /** JNI dll path. */
+        private static readonly string JniDllPath;
+
+        /** JNI dll pointer. */
+        private static readonly IntPtr JniDllPtr;
+
+        /** JNI dll finalizer. */
+        private static readonly Finalizer JniDllFinalizer;
+
         /** Interop factory ID for .Net. */
         private const int InteropFactoryId = 1;
 
@@ -42,24 +51,41 @@ namespace Apache.Ignite.Core.Impl.Unmanaged
 
             var resName = string.Format("{0}.{1}", platform, 
IgniteUtils.FileIgniteJniDll);
 
-            var path = IgniteUtils.UnpackEmbeddedResource(resName, 
IgniteUtils.FileIgniteJniDll);
+            JniDllPath = IgniteUtils.UnpackEmbeddedResource(resName, 
IgniteUtils.FileIgniteJniDll);
 
-            var ptr = NativeMethods.LoadLibrary(path);
+            JniDllPtr = NativeMethods.LoadLibrary(JniDllPath);
 
-            if (ptr == IntPtr.Zero)
+            if (JniDllPtr == IntPtr.Zero)
             {
                 var err = Marshal.GetLastWin32Error();
 
                 throw new IgniteException(string.Format("Failed to load {0} 
from {1}: [{2}]",
-                    IgniteUtils.FileIgniteJniDll, path, 
IgniteUtils.FormatWin32Error(err)));
+                    IgniteUtils.FileIgniteJniDll, JniDllPath, 
IgniteUtils.FormatWin32Error(err)));
             }
 
+            JniDllFinalizer = new Finalizer();
+
+            AppDomain.CurrentDomain.ProcessExit += CurrentDomain_ProcessExit;
+
+            // Note: this event is never called for the default AppDomain.
             AppDomain.CurrentDomain.DomainUnload += CurrentDomain_DomainUnload;
 
             JNI.SetConsoleHandler(UnmanagedCallbacks.ConsoleWriteHandler);
         }
 
         /// <summary>
+        /// Handles the ProcessExit event of the current AppDomain.
+        /// </summary>
+        private static void CurrentDomain_ProcessExit(object sender, EventArgs 
e)
+        {
+            // We unload ignite.jni.dll both in ProcessExit and Finalizer:
+            // ProcessExit is not called from custom domain (NUnit does this, 
for example).
+            // DomainUnload can't be used because it fires before all 
UnmanagedTargets are released.
+            GC.SuppressFinalize(JniDllFinalizer);
+            IgniteUtils.UnloadJniDllAndRemoveTempDirectory();
+        }
+
+        /// <summary>
         /// Handles the DomainUnload event of the current AppDomain.
         /// </summary>
         private static void CurrentDomain_DomainUnload(object sender, 
EventArgs e)
@@ -242,5 +268,22 @@ namespace Apache.Ignite.Core.Impl.Unmanaged
         }
 
         #endregion
+
+        /// <summary>
+        /// A trick to clean up ignite.jni.dll when Ignite is started in 
non-default AppDomain.
+        /// We rely on finalizer order here, which is technically undefined:
+        /// everything that uses UnmanagedUtils must be cleaned up when this 
class is finalized.
+        /// </summary>
+        private class Finalizer
+        {
+            /// <summary>
+            /// Finalizes an instance of the <see cref="Finalizer"/> class.
+            /// </summary>
+            ~Finalizer()
+            {
+                // Clean up ignite.jni.dll for the current domain.
+                IgniteUtils.UnloadJniDllAndRemoveTempDirectory(JniDllPtr, 
JniDllPath);
+            }
+        }
     }
 }

Reply via email to