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.

diff --git a/src/installer/corehost/cli/apphost/bundle/runner.cpp b/src/installer/corehost/cli/apphost/bundle/runner.cpp
index ccae5ed294996af36d467ed8ca4e3ef8da449574..14862046a95a1aa04c52b6ce19ec37b2bcaac51f 100644 (file)
--- a/src/installer/corehost/cli/apphost/bundle/runner.cpp
+++ b/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();
 

diff --git a/src/installer/corehost/cli/hostmisc/pal.unix.cpp b/src/installer/corehost/cli/hostmisc/pal.unix.cpp
index 2c60b5b46fe02c557b598eec14e628fa5ea63778..54488e6bfddaf32fec2cc5061199b78aa0e19800 100644 (file)
--- a/src/installer/corehost/cli/hostmisc/pal.unix.cpp
+++ b/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;
     }

diff --git a/src/installer/corehost/cli/hostmisc/pal.windows.cpp b/src/installer/corehost/cli/hostmisc/pal.windows.cpp
index 54b9a529ac28974552bd53c8a72108ed04b5006e..1e0ff1d985b49138772c19b7ad3abc187c343bc6 100644 (file)
--- a/src/installer/corehost/cli/hostmisc/pal.windows.cpp
+++ b/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;
 }
 

diff --git a/src/installer/test/Assets/TestProjects/AppWithWait/AppWithWait.csproj b/src/installer/test/Assets/TestProjects/AppWithWait/AppWithWait.csproj
new file mode 100644 (file)
index 0000000..59e3b57
--- /dev/null
+++ b/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>

diff --git a/src/installer/test/Assets/TestProjects/AppWithWait/Program.cs b/src/installer/test/Assets/TestProjects/AppWithWait/Program.cs
new file mode 100644 (file)
index 0000000..7a39929
--- /dev/null
+++ b/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!");
+        }
+    }
+}

diff --git a/src/installer/test/Microsoft.NET.HostModel.Tests/AppHost.Bundle.Tests/BundleRename.cs b/src/installer/test/Microsoft.NET.HostModel.Tests/AppHost.Bundle.Tests/BundleRename.cs
new file mode 100644 (file)
index 0000000..07efcce
--- /dev/null
+++ b/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();
+            }
+        }
+    }
+}

diff --git a/src/installer/test/Microsoft.NET.HostModel.Tests/Helpers/BundleHelper.cs b/src/installer/test/Microsoft.NET.HostModel.Tests/Helpers/BundleHelper.cs
index 5d245740edb6ac29f8b1261cc4e6100ef0976053..81b8a7ef2ed9908b57d9b57fb42df446ce53c939 100644 (file)
--- a/src/installer/test/Microsoft.NET.HostModel.Tests/Helpers/BundleHelper.cs
+++ b/src/installer/test/Microsoft.NET.HostModel.Tests/Helpers/BundleHelper.cs
@@ -22,6 +22,11 @@ namespace BundleTests.Helpers
             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);

diff --git a/src/installer/test/TestUtils/TestProjectFixture.cs b/src/installer/test/TestUtils/TestProjectFixture.cs
index a762d2930d431c1bd5d7b505e5d1e2938a9b055b..13f5b8b019983cd2c52acde405d6a6f20ba9010b 100644 (file)
--- a/src/installer/test/TestUtils/TestProjectFixture.cs
+++ b/src/installer/test/TestUtils/TestProjectFixture.cs
@@ -6,7 +6,6 @@ using Microsoft.DotNet.Cli.Build;
 using System;
 using System.Collections.Generic;
 using System.IO;
-using System.Text;
 
 namespace Microsoft.DotNet.CoreSetup.Test
 {
@@ -252,7 +251,8 @@ namespace Microsoft.DotNet.CoreSetup.Test
             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 @@ namespace Microsoft.DotNet.CoreSetup.Test
                 publishArgs.Add(outputDirectory);
             }
 
+            if (singleFile)
+            {
+                publishArgs.Add("/p:PublishSingleFile=true");
+            }
+
             publishArgs.Add($"/p:TestTargetRid={RepoDirProvider.TargetRID}");
             publishArgs.Add($"/p:MNAVersion={RepoDirProvider.MicrosoftNETCoreAppVersion}");