SingleFile bundles: Ensure extraction mappings are closed on Windows. (…

…#2272)

When running a single-file app, the AppHost mmap()s itself in order
to read its contents and extract the embedded contents.

The Apphost must always map its contents in order to read the headers,
but doesn't always extract the contents, because previously extracted
files are re-used when available.

In the case where apphost doesn't extract, the file mapping wasn't
immediately closed on Windows. This prevents the app from being renamed
while running -- an idiom used while updating the app in-place.

src/installer/corehost/cli/apphost/bundle/runner.cpp

@@ -50,14 +50,12 @@ StatusCode runner_t::extract()
         m_extraction_dir = extractor.extraction_dir();
 
         // Determine if embedded files are already extracted, and available for reuse
-        if (extractor.can_reuse_extraction())
+        if (!extractor.can_reuse_extraction())
         {
-            return StatusCode::Success;
-        }
-
-        manifest_t manifest = manifest_t::read(reader, header.num_embedded_files());
+            manifest_t manifest = manifest_t::read(reader, header.num_embedded_files());
 
-        extractor.extract(manifest, reader);
+            extractor.extract(manifest, reader);
+        }
 
         unmap_host();
 

src/installer/corehost/cli/hostmisc/pal.unix.cpp

@@ -63,14 +63,14 @@ void* pal::map_file_readonly(const pal::string_t& path, size_t& length)
     int fd = open(path.c_str(), O_RDONLY, (S_IRUSR | S_IRGRP | S_IROTH));
     if (fd == -1)
     {
-        trace::warning(_X("Failed to map file. open(%s) failed with error %d"), path.c_str(), errno);
+        trace::error(_X("Failed to map file. open(%s) failed with error %d"), path.c_str(), errno);
         return nullptr;
     }
 
     struct stat buf;
     if (fstat(fd, &buf) != 0)
     {
-        trace::warning(_X("Failed to map file. fstat(%s) failed with error %d"), path.c_str(), errno);
+        trace::error(_X("Failed to map file. fstat(%s) failed with error %d"), path.c_str(), errno);
         close(fd);
         return nullptr;
     }
@@ -80,7 +80,7 @@ void* pal::map_file_readonly(const pal::string_t& path, size_t& length)
 
     if(address == nullptr)
     {
-        trace::warning(_X("Failed to map file. mmap(%s) failed with error %d"), path.c_str(), errno);
+        trace::error(_X("Failed to map file. mmap(%s) failed with error %d"), path.c_str(), errno);
         close(fd);
         return nullptr;
     }

src/installer/corehost/cli/hostmisc/pal.windows.cpp

@@ -82,14 +82,14 @@ void* pal::map_file_readonly(const pal::string_t& path, size_t &length)
 
     if (file == INVALID_HANDLE_VALUE)
     {
-        trace::warning(_X("Failed to map file. CreateFileW(%s) failed with error %d"), path.c_str(), GetLastError());
+        trace::error(_X("Failed to map file. CreateFileW(%s) failed with error %d"), path.c_str(), GetLastError());
         return nullptr;
     }
 
     LARGE_INTEGER fileSize;
     if (GetFileSizeEx(file, &fileSize) == 0)
     {
-        trace::warning(_X("Failed to map file. GetFileSizeEx(%s) failed with error %d"), path.c_str(), GetLastError());
+        trace::error(_X("Failed to map file. GetFileSizeEx(%s) failed with error %d"), path.c_str(), GetLastError());
         CloseHandle(file);
         return nullptr;
     }
@@ -99,20 +99,24 @@ void* pal::map_file_readonly(const pal::string_t& path, size_t &length)
 
     if (map == NULL)
     {
-        trace::warning(_X("Failed to map file. CreateFileMappingW(%s) failed with error %d"), path.c_str(), GetLastError());
+        trace::error(_X("Failed to map file. CreateFileMappingW(%s) failed with error %d"), path.c_str(), GetLastError());
         CloseHandle(file);
         return nullptr;
     }
 
     void *address = MapViewOfFile(map, FILE_MAP_READ, 0, 0, 0);
 
-    if (map == NULL)
+    if (address == NULL)
     {
-        trace::warning(_X("Failed to map file. MapViewOfFile(%s) failed with error %d"), path.c_str(), GetLastError());
-        CloseHandle(file);
-        return nullptr;
+        trace::error(_X("Failed to map file. MapViewOfFile(%s) failed with error %d"), path.c_str(), GetLastError());
     }
 
+    // The file-handle (file) and mapping object handle (map) can be safely closed
+    // once the file is mapped. The OS keeps the file open if there is an open mapping into the file.
+
+    CloseHandle(map);
+    CloseHandle(file);
+
     return address;
 }
 

src/installer/test/Assets/TestProjects/AppWithWait/AppWithWait.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>$(NETCoreAppFramework)</TargetFramework>
+    <OutputType>Exe</OutputType>
+    <RuntimeIdentifier>$(TestTargetRid)</RuntimeIdentifier>
+    <RuntimeFrameworkVersion>$(MNAVersion)</RuntimeFrameworkVersion>
+  </PropertyGroup>
+
+</Project>

src/installer/test/Assets/TestProjects/AppWithWait/Program.cs

@@ -0,0 +1,35 @@
+using System;
+using System.IO;
+using System.Threading;
+
+namespace AppWithSubDirs
+{
+    public static class Program
+    {
+        public static void Main(string[] args)
+        {
+            Console.Write("Hello ");
+
+            // If the caller wants the app to start and wait,
+            // it provides the name of a lock-file to write.
+            // In this case, this test app creates the lock file
+            // and waits until the file is deleted.
+            if (args.Length > 0)
+            {
+                string writeFile = args[0];
+
+                var fs = File.Create(writeFile);
+                fs.Close();
+
+                Thread.Sleep(200);
+
+                while (File.Exists(writeFile))
+                {
+                    Thread.Sleep(100);
+                }
+            }
+
+            Console.WriteLine("World!");
+        }
+    }
+}

src/installer/test/Microsoft.NET.HostModel.Tests/AppHost.Bundle.Tests/BundleRename.cs

@@ -0,0 +1,91 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.IO;
+using Xunit;
+using Microsoft.DotNet.Cli.Build.Framework;
+using Microsoft.DotNet.CoreSetup.Test;
+using BundleTests.Helpers;
+using System.Threading;
+
+namespace AppHost.Bundle.Tests
+{
+    public class BundleRename : IClassFixture<BundleRename.SharedTestState>
+    {
+        private SharedTestState sharedTestState;
+
+        public BundleRename(SharedTestState fixture)
+        {
+            sharedTestState = fixture;
+        }
+
+        [Theory]
+        [InlineData(true)]  // Test renaming the single-exe during the initial run, when contents are extracted
+        [InlineData(false)] // Test renaming the single-exe during subsequent runs, when contents are reused
+        private void Bundle_can_be_renamed_while_running(bool renameFirstRun)
+        {
+            var fixture = sharedTestState.TestFixture.Copy();
+            string singleFile = BundleHelper.GetPublishedSingleFilePath(fixture);
+            string renameFile = Path.Combine(BundleHelper.GetPublishPath(fixture), Path.GetRandomFileName());
+            string writeFile = Path.Combine(BundleHelper.GetPublishPath(fixture), "lock");
+
+            if (!renameFirstRun)
+            {
+                Command.Create(singleFile)
+                    .CaptureStdErr()
+                    .CaptureStdOut()
+                    .Execute()
+                    .Should()
+                    .Pass()
+                    .And
+                    .HaveStdOutContaining("Hello World!");
+            }
+
+            var singleExe = Command.Create(singleFile, writeFile)
+                .CaptureStdErr()
+                .CaptureStdOut()
+                .Start();
+
+            // Once the App starts running, it creates the writeFile, and waits until the file is deleted.
+            while (!File.Exists(writeFile))
+            {
+                Thread.Sleep(100);
+            }
+
+            File.Move(singleFile, renameFile);
+            File.Delete(writeFile);
+
+            var result = singleExe.WaitForExit(fExpectedToFail: false);
+
+            result
+                .Should()
+                .Pass()
+                .And
+                .HaveStdOutContaining("Hello World!");
+        }
+
+        public class SharedTestState : IDisposable
+        {
+            public TestProjectFixture TestFixture { get; set; }
+            public RepoDirectoriesProvider RepoDirectories { get; set; }
+
+            public SharedTestState()
+            {
+                RepoDirectories = new RepoDirectoriesProvider();
+                TestFixture = new TestProjectFixture("AppWithWait", RepoDirectories);
+                TestFixture
+                    .EnsureRestoredForRid(TestFixture.CurrentRid, RepoDirectories.CorehostPackages)
+                    .PublishProject(runtime: TestFixture.CurrentRid,
+                                    singleFile: true,
+                                    outputDirectory: BundleHelper.GetPublishPath(TestFixture));
+            }
+
+            public void Dispose()
+            {
+                TestFixture.Dispose();
+            }
+        }
+    }
+}

src/installer/test/Microsoft.NET.HostModel.Tests/Helpers/BundleHelper.cs

@@ -22,6 +22,11 @@ public static string GetAppPath(TestProjectFixture fixture)
             return Path.Combine(GetPublishPath(fixture), GetAppName(fixture));
         }
 
+        public static string GetPublishedSingleFilePath(TestProjectFixture fixture)
+        {
+            return GetHostPath(fixture);
+        }
+
         public static string GetHostName(TestProjectFixture fixture)
         {
             return Path.GetFileName(fixture.TestProject.AppExe);

src/installer/test/TestUtils/TestProjectFixture.cs

@@ -6,7 +6,6 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
-using System.Text;
 
 namespace Microsoft.DotNet.CoreSetup.Test
 {
@@ -252,7 +251,8 @@ public TestProjectFixture PublishProject(
             string runtime = null,
             string framework = null,
             string selfContained = null,
-            string outputDirectory = null)
+            string outputDirectory = null,
+            bool singleFile = false)
         {
             dotnet = dotnet ?? SdkDotnet;
             outputDirectory = outputDirectory ?? TestProject.OutputDirectory;
@@ -291,6 +291,11 @@ public TestProjectFixture PublishProject(
                 publishArgs.Add(outputDirectory);
             }
 
+            if (singleFile)
+            {
+                publishArgs.Add("/p:PublishSingleFile=true");
+            }
+
             publishArgs.Add($"/p:TestTargetRid={RepoDirProvider.TargetRID}");
             publishArgs.Add($"/p:MNAVersion={RepoDirProvider.MicrosoftNETCoreAppVersion}");