[release/3.1] Single file: Guard against partial cleanup of extracted files

** Issue
dotnet/runtime#3778

** Customer Scenario

Single-file apps rely on extractiong their contents to disk.
The extaction is performed to a temp-directory by default, which may be cleaned up by the OS.

The app can re-extract itself if the entire extraction is cleaned up -- but not partial deletions.
Customers have noticed that for some apps, the extracted files were removed, but the extraction directory itself wasn't.
This causes the app to fail to start.

** Problem

When executing single-file apps, if a pre-existing extraction exists, it is assumed to be valid.

** Solution

When executing single-file apps, if a pre-existing extraction exists, the contents of the extraction directory are now verified.
If files are missing, they are recovered.

**Extraction Algorithm**

`ExtractionDir` = `$DOTNET_BUNDLE_EXTRACT_BASE_DIR/<app>/<bundle-id>`
`WorkingDir` = `$DOTNET_BUNDLE_EXTRACT_BASE_DIR/<app>/<process-id>`

If `ExtractionDir` does not exist, then

Create `WorkingDir`, and extract bundle contents within it
Attempt to rename `WorkingDir` as `ExtractionDir`
If the rename succeeds, continue execution of the app
If not, another process completed extraction; Verify and reuse this extraction (as in step 2).
If `ExtractionDir` exists, then verify that it contains all files listed in the manifest.

If all contents are available,
Remove `WorkingDir`
Continue execution of the app, reusing the contents.

If certain files are missing within `ExtractionDir`, then
For each missing file, do the following individually
  Extract the files within `WorkingDir`
  Create sub-paths within `ExtractionDir` if necessary
  Rename the file from `WorkingDir/path/<file>` to `ExtractionDir/path/<file>` unless `ExtractionDir/path/<file>` exists (extracted there by another process in the meantime)
Remove `WorkingDir`
Continue execution of the app.

All of the renames above are done with appropriate retries to circumvent interference from anti-virus apps.

** Startup time impact
* Console HelloWorld execution time:
    * Framework dependent app: Windows/Linux No measurable difference
    * Self-contained app: Windows: ~10ms additional
    * Self-contained app: Linux: No measurable difference
* Greenshot Startup:
    * Self-contained Windows: No noticiable/measurable difference
* NugetPackageExplorer Startup:
    * Self-contained Windows: No noticiable/measurable difference

** Risk

Medium.
The change is well scoped, but isn't trivial.
It affects all single-file apps.

** Master Branch
dotnet/runtime#32649

Author: swaroop-sridhar

src/corehost/cli/apphost/bundle/bundle_runner.cpp

@@ -135,17 +135,23 @@ static void remove_directory_tree(const pal::string_t& path)
 
     for (const pal::string_t &dir : dirs)
     {
-        remove_directory_tree(dir);
+        pal::string_t dir_path = path;
+        append_path(&dir_path, dir.c_str());
+
+        remove_directory_tree(dir_path);
     }
 
     std::vector<pal::string_t> files;
     pal::readdir(path, &files);
 
     for (const pal::string_t &file : files)
     {
-        if (!pal::remove(file.c_str()))
+        pal::string_t file_path = path;
+        append_path(&file_path, file.c_str());
+
+        if (!pal::remove(file_path.c_str()))
         {
-            trace::warning(_X("Failed to remove temporary file [%s]."), file.c_str());
+            trace::warning(_X("Failed to remove temporary file [%s]."), file_path.c_str());
         }
     }
 
@@ -155,6 +161,49 @@ static void remove_directory_tree(const pal::string_t& path)
     }
 }
 
+// Retry the rename operation with some wait in between the attempts.
+// This is an attempt to workaround for possible file locking caused by AV software.
+
+bool rename_with_retries(pal::string_t& old_name, pal::string_t& new_name, bool& file_exists)
+{
+    for (int retry_count = 0; retry_count < 500; retry_count++)
+    {
+        if (pal::rename(old_name.c_str(), new_name.c_str()) == 0)
+        {
+            return true;
+        }
+        bool should_retry = errno == EACCES;
+
+        if (pal::file_exists(new_name))
+        {
+            // Check file_exists() on each run, because a concurrent process may have
+            // created the new_name file/directory.
+            //
+            // The rename() operation above fails with errono == EACCESS if 
+            // * Directory new_name already exists, or
+            // * Paths are invalid paths, or
+            // * Due to locking/permission problems.
+            // Therefore, we need to perform the directory_exists() check again.
+
+            file_exists = true;
+            return false;
+        }
+
+        if (should_retry)
+        {
+            trace::info(_X("Retrying Rename [%s] to [%s] due to EACCES error"), old_name.c_str(), new_name.c_str());
+            pal::sleep(100);
+            continue;
+        }
+        else
+        {
+            return false;
+        }
+    }
+
+    return false;
+}
+
 void bundle_runner_t::reopen_host_for_reading()
 {
     m_bundle_stream = pal::file_open(m_bundle_path, _X("rb"));
@@ -254,18 +303,98 @@ void bundle_runner_t::extract_file(file_entry_t *entry)
     fclose(file);
 }
 
-bool bundle_runner_t::can_reuse_extraction()
+void bundle_runner_t::commit_file(const pal::string_t& relative_path)
 {
-    // In this version, the extracted files are assumed to be 
-    // correct by construction.
-    // 
-    // Files embedded in the bundle are first extracted to m_working_extraction_dir
-    // Once all files are successfully extracted, the extraction location is 
-    // committed (renamed) to m_extraction_dir. Therefore, the presence of 
-    // m_extraction_dir means that the files are pre-extracted. 
+    // Commit individual files to the final extraction directory.
+
+    pal::string_t working_file_path = m_working_extraction_dir;
+    append_path(&working_file_path, relative_path.c_str());
 
+    pal::string_t final_file_path = m_extraction_dir;
+    append_path(&final_file_path, relative_path.c_str());
 
-    return pal::directory_exists(m_extraction_dir);
+    if (has_dirs_in_path(relative_path))
+    {
+        create_directory_tree(get_directory(final_file_path));
+    }
+
+    bool extracted_by_concurrent_process = false;
+    bool extracted_by_current_process =
+        rename_with_retries(working_file_path, final_file_path, extracted_by_concurrent_process);
+
+    if (extracted_by_concurrent_process)
+    {
+        // Another process successfully extracted the dependencies
+        trace::info(_X("Extraction completed by another process, aborting current extraction."));
+    }
+
+    if (!extracted_by_current_process && !extracted_by_concurrent_process)
+    {
+        trace::error(_X("Failure processing application bundle."));
+        trace::error(_X("Failed to commit extracted files to directory [%s]."), m_extraction_dir.c_str());
+        throw StatusCode::BundleExtractionFailure;
+    }
+
+    trace::info(_X("Extraction recovered [%s]"), relative_path.c_str());
+}
+
+void bundle_runner_t::commit_dir()
+{
+    // Commit an entire new extraction to the final extraction directory
+    // Retry the move operation with some wait in between the attempts. This is to workaround for possible file locking
+    // caused by AV software. Basically the extraction process above writes a bunch of executable files to disk
+    // and some AV software may decide to scan them on write. If this happens the files will be locked which blocks
+    // our ablity to move them.
+
+    bool extracted_by_concurrent_process = false;
+    bool extracted_by_current_process =
+        rename_with_retries(m_working_extraction_dir, m_extraction_dir, extracted_by_concurrent_process);
+
+    if (extracted_by_concurrent_process)
+    {
+        // Another process successfully extracted the dependencies
+        trace::info(_X("Extraction completed by another process, aborting current extraction."));
+        remove_directory_tree(m_working_extraction_dir);
+    }
+
+    if (!extracted_by_current_process && !extracted_by_concurrent_process)
+    {
+        trace::error(_X("Failure processing application bundle."));
+        trace::error(_X("Failed to commit extracted files to directory [%s]."), m_extraction_dir.c_str());
+        throw StatusCode::BundleExtractionFailure;
+    }
+
+    trace::info(_X("Completed new extraction."));
+}
+
+// Verify that an existing extraction contains all files listed in the bundle manifest.
+// If some files are missing, extract them individually.
+void bundle_runner_t::verify_recover_extraction()
+{
+    bool recovered = false;
+
+    for (file_entry_t* entry : m_manifest->files)
+    {
+        pal::string_t file_path = m_extraction_dir;
+        append_path(&file_path, entry->relative_path().c_str());
+
+        if (!pal::file_exists(file_path))
+        {
+            if (!recovered)
+            {
+                recovered = true;
+                create_working_extraction_dir();
+            }
+
+            extract_file(entry);
+            commit_file(entry->relative_path());
+        }
+    }
+
+    if (recovered)
+    {
+        remove_directory_tree(m_working_extraction_dir);
+    }
 }
 
 // Current support for executing single-file bundles involves 
@@ -281,13 +410,22 @@ StatusCode bundle_runner_t::extract()
         seek(m_bundle_stream, marker_t::header_offset(), SEEK_SET);
         m_header.reset(header_t::read(m_bundle_stream));
 
+        m_manifest.reset(manifest_t::read(m_bundle_stream, num_embedded_files()));
+
         // Determine if embedded files are already extracted, and available for reuse
         determine_extraction_dir();
-        if (can_reuse_extraction())
+        if (pal::directory_exists(m_extraction_dir))
         {
+            // If so, verify the contents, recover files if necessary, 
+            // and use the existing extraction.
+
+            verify_recover_extraction();
+            fclose(m_bundle_stream);
             return StatusCode::Success;
         }
 
+        // If not, create a new extraction
+
         // Extract files to temporary working directory
         //
         // Files are extracted to a specific deterministic location on disk
@@ -307,46 +445,11 @@ StatusCode bundle_runner_t::extract()
 
         create_working_extraction_dir();
 
-        m_manifest.reset(manifest_t::read(m_bundle_stream, num_embedded_files()));
-
         for (file_entry_t* entry : m_manifest->files) {
             extract_file(entry);
         }
 
-        // Commit files to the final extraction directory
-        // Retry the move operation with some wait in between the attempts. This is to workaround for possible file locking
-        // caused by AV software. Basically the extraction process above writes a bunch of executable files to disk
-        // and some AV software may decide to scan them on write. If this happens the files will be locked which blocks
-        // our ablity to move them.
-        int retry_count = 500;
-        while (true)
-        {
-            if (pal::rename(m_working_extraction_dir.c_str(), m_extraction_dir.c_str()) == 0)
-                break;
-
-            bool should_retry = errno == EACCES;
-            if (can_reuse_extraction())
-            {
-                // Another process successfully extracted the dependencies
-                trace::info(_X("Extraction completed by another process, aborting current extraction."));
-
-                remove_directory_tree(m_working_extraction_dir);
-                break;
-            }
-
-            if (should_retry && (retry_count--) > 0)
-            {
-                trace::info(_X("Retrying extraction due to EACCES trying to rename the extraction folder to [%s]."), m_extraction_dir.c_str());
-                pal::sleep(100);
-                continue;
-            }
-            else
-            {
-                trace::error(_X("Failure processing application bundle."));
-                trace::error(_X("Failed to commit extracted files to directory [%s]"), m_extraction_dir.c_str());
-                throw StatusCode::BundleExtractionFailure;
-            }
-        }
+        commit_dir();
 
         fclose(m_bundle_stream);
         return StatusCode::Success;

src/corehost/cli/apphost/bundle/bundle_runner.h

@@ -45,7 +45,9 @@ namespace bundle
 
         void determine_extraction_dir();
         void create_working_extraction_dir();
-        bool can_reuse_extraction();
+        void verify_recover_extraction();
+        void commit_file(const pal::string_t& relative_path);
+        void commit_dir();
 
         FILE* create_extraction_file(const pal::string_t& relative_path);
         void extract_file(file_entry_t* entry);

src/test/BundleTests/AppHost.Bundle.Tests/BundleExtractToSpecificPath.cs

@@ -118,6 +118,57 @@ private void Bundle_extraction_is_reused()
             extractDir.Should().NotBeModifiedAfter(firstWriteTime);
         }
 
+        [Fact]
+        private void Bundle_extraction_can_recover_missing_files()
+        {
+            var fixture = sharedTestState.TestFixture.Copy();
+            var hostName = BundleHelper.GetHostName(fixture);
+            var appName = Path.GetFileNameWithoutExtension(hostName);
+            string publishPath = BundleHelper.GetPublishPath(fixture);
+
+            // Publish the bundle
+            var bundleDir = BundleHelper.GetBundleDir(fixture);
+            var bundler = new Microsoft.NET.HostModel.Bundle.Bundler(hostName, bundleDir.FullName);
+            string singleFile = bundler.GenerateBundle(publishPath);
+
+            // Compute bundled files
+            List<string> bundledFiles = bundler.BundleManifest.Files.Select(file => file.RelativePath).ToList();
+
+            // Create a directory for extraction.
+            var extractBaseDir = BundleHelper.GetExtractDir(fixture);
+            string extractPath = Path.Combine(extractBaseDir.FullName, appName, bundler.BundleManifest.BundleID);
+            var extractDir = new DirectoryInfo(extractPath);
+
+            // Run the bunded app for the first time, and extract files to 
+            // $DOTNET_BUNDLE_EXTRACT_BASE_DIR/<app>/bundle-id
+            Command.Create(singleFile)
+                .CaptureStdErr()
+                .CaptureStdOut()
+                .EnvironmentVariable(BundleHelper.DotnetBundleExtractBaseEnvVariable, extractBaseDir.FullName)
+                .Execute()
+                .Should()
+                .Pass()
+                .And
+                .HaveStdOutContaining("Hello World");
+
+            bundledFiles.ForEach(file => File.Delete(Path.Combine(extractPath, file)));
+
+            extractDir.Should().Exist();
+            extractDir.Should().NotHaveFiles(bundledFiles);
+
+            // Run the bundled app again (recover deleted files)
+            Command.Create(singleFile)
+                .CaptureStdErr()
+                .CaptureStdOut()
+                .EnvironmentVariable(BundleHelper.DotnetBundleExtractBaseEnvVariable, extractBaseDir.FullName)
+                .Execute()
+                .Should()
+                .Pass()
+                .And
+                .HaveStdOutContaining("Hello World");
+
+            extractDir.Should().HaveFiles(bundledFiles);
+        }
 
         public class SharedTestState : IDisposable
         {