StringAsTempFile cleans up in a wider range of circumstances (not rel…

…ying on finalizer running). Helps with #7 but still doesn't cover all cases.
dotnet · Nov 30, 2016 · 2cffab1 · 2cffab1
1 parent 9001c19
commit 2cffab1
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 3 deletions.
diff --git a/src/Microsoft.AspNetCore.NodeServices/Util/StringAsTempFile.cs b/src/Microsoft.AspNetCore.NodeServices/Util/StringAsTempFile.cs
@@ -9,6 +9,8 @@ namespace Microsoft.AspNetCore.NodeServices
     public sealed class StringAsTempFile : IDisposable
     {
         private bool _disposedValue;
+        private bool _hasDeletedTempFile;
+        private object _fileDeletionLock = new object();
 
         /// <summary>
         /// Create a new instance of <see cref="StringAsTempFile"/>.
@@ -18,6 +20,18 @@ public StringAsTempFile(string content)
         {
             FileName = Path.GetTempFileName();
             File.WriteAllText(FileName, content);
+
+            // Because .NET finalizers don't reliably run when the process is terminating, also
+            // add event handlers for other shutdown scenarios.
+#if NET451
+            AppDomain.CurrentDomain.ProcessExit += HandleProcessExit;
+            AppDomain.CurrentDomain.DomainUnload += HandleProcessExit;
+#else
+            // Note that this still doesn't capture SIGKILL (at least on macOS) - there doesn't
+            // appear to be a way of doing that. So in that case, the temporary file will be
+            // left behind.
+            System.Runtime.Loader.AssemblyLoadContext.Default.Unloading += HandleAssemblyUnloading;
+#endif
         }
 
         /// <summary>
@@ -40,15 +54,45 @@ private void DisposeImpl(bool disposing)
             {
                 if (disposing)
                 {
-                    // Would dispose managed state here, if there was any
+                    // Dispose managed state
+#if NET451
+                    AppDomain.CurrentDomain.ProcessExit -= HandleProcessExit;
+                    AppDomain.CurrentDomain.DomainUnload -= HandleProcessExit;
+#else
+                    System.Runtime.Loader.AssemblyLoadContext.Default.Unloading -= HandleAssemblyUnloading;
+#endif
                 }
 
-                File.Delete(FileName);
+                EnsureTempFileDeleted();
 
                 _disposedValue = true;
             }
         }
 
+        private void EnsureTempFileDeleted()
+        {
+            lock (_fileDeletionLock)
+            {
+                if (!_hasDeletedTempFile)
+                {
+                    File.Delete(FileName);
+                    _hasDeletedTempFile = true;
+                }
+            }
+        }
+
+#if NET451
+        private void HandleProcessExit(object sender, EventArgs args)
+        {
+            EnsureTempFileDeleted();
+        }
+#else
+        private void HandleAssemblyUnloading(System.Runtime.Loader.AssemblyLoadContext context)
+        {
+            EnsureTempFileDeleted();
+        }
+#endif
+
         /// <summary>
         /// Implements the finalization part of the IDisposable pattern by calling Dispose(false).
         /// </summary>

diff --git a/src/Microsoft.AspNetCore.NodeServices/project.json b/src/Microsoft.AspNetCore.NodeServices/project.json
@@ -31,7 +31,8 @@
     "netstandard1.6": {
       "dependencies": {
         "System.Diagnostics.Process": "4.3.0",
-        "System.IO.FileSystem.Watcher": "4.3.0"
+        "System.IO.FileSystem.Watcher": "4.3.0",
+        "System.Runtime.Loader": "4.3.0"
       }
     }
   },