Merge pull request microsoft#1976 from tyrielv/tyrielv/fix-unmountall-upgrade-check

Apply staged upgrade when mount processes exit

Author: tyrielv

.github/workflows/upgrade-tests.yaml

@@ -33,6 +33,7 @@ jobs:
           - double-staging
           - staging-then-clean
           - mount-safety-deferral
+          - unmount-all-triggers-upgrade
       fail-fast: false
 
     steps:
@@ -274,6 +275,36 @@ jobs:
             Write-Host "PASS: Mount safety deferral works correctly"
           }
 
+          "unmount-all-triggers-upgrade" {
+            Write-Host "=== Scenario: unmount-all triggers staged upgrade ==="
+            # Install LKG, mount, staging upgrade with new installer (which
+            # replaces GVFS.Service.exe in-place with the new version that
+            # includes PendingUpgradeMonitor). Then unmount via --unmount-all.
+            # The new service monitors mount process exits and applies the
+            # upgrade automatically — no pipe message from gvfs.exe needed.
+            Install-GVFS $lkgInstaller
+            Assert-ServiceRunning
+            $mountPid = Mount-TestRepo
+
+            Install-GVFS $newInstaller @("/STAGEIFMOUNTED=true")
+            Assert-MountAlive $mountPid
+            Assert-PendingUpgrade $true
+
+            # Unmount via --unmount-all (uses LKG gvfs.exe — no new pipe msg)
+            & "$installDir\gvfs.exe" service --unmount-all 2>&1 | Write-Host
+            if ($LASTEXITCODE -ne 0) { throw "unmount-all failed" }
+
+            # The monitor's debounce timer fires 5s after the last mount
+            # process exits, then applies the upgrade. Wait for completion.
+            $deadline = (Get-Date).AddSeconds(30)
+            while ((Test-Path "$installDir\PendingUpgrade") -and (Get-Date) -lt $deadline) {
+              Start-Sleep -Seconds 2
+            }
+
+            Assert-PendingUpgrade $false
+            Write-Host "PASS: unmount-all triggers staged upgrade via process monitor"
+          }
+
           default {
             throw "Unknown scenario: ${{ matrix.scenario }}"
           }

GVFS/GVFS.Service/GVFSService.Windows.cs

@@ -26,6 +26,7 @@ public class GVFSService : ServiceBase
         private RepoRegistry repoRegistry;
         private WindowsRequestHandler requestHandler;
         private INotificationHandler notificationHandler;
+        private PendingUpgradeMonitor pendingUpgradeMonitor;
 
         public GVFSService(JsonTracer tracer)
         {
@@ -46,8 +47,14 @@ public void Run()
                 // Check for a staged upgrade before doing anything else.
                 // If no GVFS.Mount processes are running (typical at boot or after
                 // unmount-all), copy staged files in-place and proceed normally.
-                // If mounts ARE running, the upgrade is deferred to next restart.
-                PendingUpgradeHandler.TryApplyPendingUpgrade(this.tracer);
+                // If mounts ARE running, start a monitor that will apply the
+                // upgrade when all mount processes exit.
+                UpgradeResult upgradeResult = PendingUpgradeHandler.TryApplyPendingUpgrade(this.tracer);
+                if (upgradeResult == UpgradeResult.DeferredMountsRunning)
+                {
+                    this.pendingUpgradeMonitor = new PendingUpgradeMonitor(this.tracer);
+                    this.pendingUpgradeMonitor.Start();
+                }
 
                 this.repoRegistry = new RepoRegistry(
                     this.tracer,
@@ -99,6 +106,12 @@ public void StopRunning()
                     this.tracer.RelatedInfo("Stopping");
                 }
 
+                if (this.pendingUpgradeMonitor != null)
+                {
+                    this.pendingUpgradeMonitor.Dispose();
+                    this.pendingUpgradeMonitor = null;
+                }
+
                 if (this.serviceStopped != null)
                 {
                     this.serviceStopped.Set();

GVFS/GVFS.Service/Handlers/RequestHandler.cs

@@ -15,7 +15,7 @@ namespace GVFS.Service.Handlers
     /// </summary>
     public class RequestHandler
     {
-        private const int PendingUpgradeDelayMs = 5000;
+        private const int PendingUpgradeDelayMs = 2000;
 
         protected const string EnableProjFSRequestDescription = "attach volume";
         protected string requestDescription;

GVFS/GVFS.Service/PendingUpgradeHandler.cs

@@ -14,8 +14,9 @@ namespace GVFS.Service
     /// When the installer runs with mounts active, it stages new files to
     /// {installDir}\PendingUpgrade\ instead of replacing files in-place.
     /// This class applies the upgrade when no GVFS.Mount processes are
-    /// running — either on service start (before automount) or after a
-    /// repo unmount (via deferred check from RequestHandler).
+    /// running — either on service start (before automount), after a
+    /// repo unmount (via deferred check from RequestHandler), or when
+    /// PendingUpgradeMonitor detects all mount processes have exited.
     ///
     ///   1. Move old files from install dir → PreviousVersion\
     ///   2. Move new files from PendingUpgrade\ → install dir
@@ -36,6 +37,9 @@ public static class PendingUpgradeHandler
         private const string Phase1CompleteMarkerFileName = ".phase1-complete";
         private const string ServiceExeName = "GVFS.Service.exe";
         private const string MountProcessName = "GVFS.Mount";
+        private const string MountExeName = "GVFS.Mount.exe";
+
+        private static readonly object ApplyLock = new object();
 
         // Executables that users or the service can launch to start new
         // mount/hook processes. During upgrade these are moved out first
@@ -53,22 +57,89 @@ public static class PendingUpgradeHandler
         /// <summary>
         /// Checks for and applies a pending staged upgrade.
         /// </summary>
-        public static void TryApplyPendingUpgrade(ITracer tracer)
+        public static UpgradeResult TryApplyPendingUpgrade(ITracer tracer)
+        {
+            lock (ApplyLock)
+            {
+                return TryApplyPendingUpgradeLocked(tracer);
+            }
+        }
+
+        /// <summary>
+        /// Returns true if a PendingUpgrade directory with a .ready marker exists.
+        /// </summary>
+        public static bool IsPending()
+        {
+            string pendingUpgradeDir = Path.Combine(Configuration.AssemblyPath, PendingUpgradeDirectoryName);
+            if (!Directory.Exists(pendingUpgradeDir))
+            {
+                return false;
+            }
+
+            string readyMarker = Path.Combine(pendingUpgradeDir, ReadyMarkerFileName);
+            return File.Exists(readyMarker);
+        }
+
+        /// <summary>
+        /// Returns GVFS.Mount processes whose executable is in the install
+        /// directory. Processes from dev builds or other installs are excluded
+        /// so they don't block upgrades of the system install. If a process's
+        /// path cannot be read (access denied, 32/64-bit mismatch), it is
+        /// included conservatively.
+        /// Caller must dispose the returned Process objects.
+        /// </summary>
+        public static List<Process> GetInstalledMountProcesses(ITracer tracer)
+        {
+            string installDir = Configuration.AssemblyPath;
+            string expectedPath = Path.Combine(installDir, MountExeName);
+            Process[] allMountProcesses = Process.GetProcessesByName(MountProcessName);
+            List<Process> installed = new List<Process>();
+
+            foreach (Process process in allMountProcesses)
+            {
+                bool include = true;
+                try
+                {
+                    string processPath = process.MainModule?.FileName;
+                    if (processPath != null &&
+                        !PathComparer.Equals(processPath, expectedPath))
+                    {
+                        include = false;
+                        tracer.RelatedInfo(
+                            $"{nameof(PendingUpgradeHandler)}: Skipping GVFS.Mount PID {process.Id} " +
+                            $"(path: {processPath}, not in install dir)");
+                    }
+                }
+                catch (Exception)
+                {
+                    // Access denied or process exited — include conservatively
+                }
+
+                if (include)
+                {
+                    installed.Add(process);
+                }
+                else
+                {
+                    process.Dispose();
+                }
+            }
+
+            return installed;
+        }
+
+        private static UpgradeResult TryApplyPendingUpgradeLocked(ITracer tracer)
         {
             string installDir = Configuration.AssemblyPath;
             string pendingUpgradeDir = Path.Combine(installDir, PendingUpgradeDirectoryName);
             string previousVersionDir = Path.Combine(installDir, PreviousVersionDirectoryName);
 
             if (!Directory.Exists(pendingUpgradeDir))
             {
-                // No pending upgrade. Clean up PreviousVersion if it exists
-                // (leftover from a completed upgrade where cleanup was interrupted).
                 TryDeleteDirectory(tracer, previousVersionDir, "leftover PreviousVersion");
-                return;
+                return UpgradeResult.NoPending;
             }
 
-            // Installer writes .ready marker as its last step. If missing,
-            // the installer was interrupted mid-write — don't apply partial files.
             string readyMarker = Path.Combine(pendingUpgradeDir, ReadyMarkerFileName);
             if (!File.Exists(readyMarker))
             {
@@ -79,28 +150,25 @@ public static void TryApplyPendingUpgrade(ITracer tracer)
                     $"{nameof(PendingUpgradeHandler)}: PendingUpgrade directory exists but {ReadyMarkerFileName} marker " +
                     "is missing — installer was likely interrupted. Skipping until next install completes.",
                     Keywords.Telemetry);
-                return;
+                return UpgradeResult.NotReady;
             }
 
             tracer.RelatedInfo($"{nameof(PendingUpgradeHandler)}: Pending upgrade detected at {pendingUpgradeDir}");
 
-            // Don't apply if GVFS.Mount processes are still running — their
-            // executables are locked and moves would fail. Upgrade will be
-            // retried on next service start when no mounts are active.
-            Process[] mountProcesses = Array.Empty<Process>();
+            List<Process> mountProcesses = new List<Process>();
             try
             {
-                mountProcesses = Process.GetProcessesByName(MountProcessName);
-                if (mountProcesses.Length > 0)
+                mountProcesses = GetInstalledMountProcesses(tracer);
+                if (mountProcesses.Count > 0)
                 {
                     EventMetadata deferMetadata = new EventMetadata();
-                    deferMetadata.Add("MountProcessCount", mountProcesses.Length);
+                    deferMetadata.Add("MountProcessCount", mountProcesses.Count);
                     tracer.RelatedEvent(
                         EventLevel.Informational,
                         $"{nameof(PendingUpgradeHandler)}_Deferred",
                         deferMetadata,
                         Keywords.Telemetry);
-                    return;
+                    return UpgradeResult.DeferredMountsRunning;
                 }
             }
             finally
@@ -217,7 +285,7 @@ public static void TryApplyPendingUpgrade(ITracer tracer)
                     $"{nameof(PendingUpgradeHandler)}_Complete",
                     successMetadata,
                     Keywords.Telemetry);
-                return;
+                return UpgradeResult.Applied;
             }
             catch (Exception ex)
             {
@@ -229,7 +297,7 @@ public static void TryApplyPendingUpgrade(ITracer tracer)
                     "PendingUpgrade retained for retry on next service start. " +
                     "If PreviousVersion exists, old files are preserved for manual recovery.",
                     Keywords.Telemetry);
-                return;
+                return UpgradeResult.Failed;
             }
         }
 
@@ -440,4 +508,13 @@ private static void TryDeleteDirectory(ITracer tracer, string path, string descr
             }
         }
     }
+
+    public enum UpgradeResult
+    {
+        NoPending,
+        Applied,
+        DeferredMountsRunning,
+        NotReady,
+        Failed,
+    }
 }