Fix staged upgrade races and installer error handling

Address review feedback from #1958:

- StopService/StagingUpdateService: check sc.exe exit code separately
  from Exec launch failure. Previously non-zero sc.exe results were
  silently ignored.

- Timer race: wrap pendingUpgradeTimer create/reset in a lock. Two
  concurrent unmounts on separate pipe threads could both observe null
  and create duplicate timers, causing parallel TryApplyPendingUpgrade.

- .ready race: move service start from AfterInstall hook to ssPostInstall
  after .ready marker is written. Previously the service could start its
  5s debounce timer before .ready existed, skip the upgrade, and leave
  staged files until the next service restart.

Assisted-by: Claude Opus 4.6
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>

Author: tyrielv

GVFS/GVFS.Installers/Setup.iss

@@ -66,7 +66,7 @@ DestDir: "{app}"; Flags: ignoreversion; Source:"{#LayoutDir}\GVFS.Service.exe";
 ; goes directly to {app} so the restarted service has PendingUpgradeHandler code.
 ; The service is briefly stopped/restarted (mounts are independent processes).
 DestDir: "{app}\PendingUpgrade"; Flags: ignoreversion recursesubdirs; Source:"{#LayoutDir}\*"; Check: IsStagingInstall
-DestDir: "{app}"; Flags: ignoreversion; Source:"{#LayoutDir}\GVFS.Service.exe"; AfterInstall: StagingUpdateService; Check: IsStagingInstall
+DestDir: "{app}"; Flags: ignoreversion; Source:"{#LayoutDir}\GVFS.Service.exe"; Check: IsStagingInstall
 
 [Dirs]
 Name: "{app}\ProgramData\{#ServiceName}"; Permissions: users-readexec
@@ -166,11 +166,17 @@ var
   ResultCode: integer;
 begin
   Log('StopService: stopping: ' + ServiceName);
-  // ErrorCode 1060 means service not installed, 1062 means service not started
-  if not Exec(ExpandConstant('{sys}\SC.EXE'), 'stop ' + ServiceName, '', SW_HIDE, ewWaitUntilTerminated, ResultCode) and (ResultCode <> 1060) and (ResultCode <> 1062) then
+  if not Exec(ExpandConstant('{sys}\SC.EXE'), 'stop ' + ServiceName, '', SW_HIDE, ewWaitUntilTerminated, ResultCode) then
     begin
+      Log('StopService: Failed to launch sc.exe');
       RaiseException('Fatal: Could not stop service: ' + ServiceName);
     end;
+  // 1060 = service not installed, 1062 = service not started
+  if (ResultCode <> 0) and (ResultCode <> 1060) and (ResultCode <> 1062) then
+    begin
+      Log('StopService: sc stop returned error code ' + IntToStr(ResultCode));
+      RaiseException('Fatal: Could not stop service: ' + ServiceName + ' (exit code ' + IntToStr(ResultCode) + ')');
+    end;
 end;
 
 procedure WaitForServiceProcessToExit(ServiceName: string);
@@ -328,9 +334,14 @@ begin
 
   try
     Log('StagingUpdateService: Starting service with new binary');
-    if not Exec(ExpandConstant('{sys}\SC.EXE'), 'start GVFS.Service', '', SW_HIDE, ewWaitUntilTerminated, ResultCode) then
+    if Exec(ExpandConstant('{sys}\SC.EXE'), 'start GVFS.Service', '', SW_HIDE, ewWaitUntilTerminated, ResultCode) then
+      begin
+        if ResultCode <> 0 then
+          Log('StagingUpdateService: Warning - sc start returned error code ' + IntToStr(ResultCode));
+      end
+    else
       begin
-        Log('StagingUpdateService: Warning - could not start service: ' + SysErrorMessage(ResultCode));
+        Log('StagingUpdateService: Warning - could not launch sc.exe');
       end;
 
     WriteOnDiskVersion16CapableFile();
@@ -721,6 +732,11 @@ begin
             // complete and safe to apply.
             SaveStringToFile(ExpandConstant('{app}\PendingUpgrade\.ready'), '', False);
             Log('CurStepChanged: Wrote PendingUpgrade .ready marker');
+
+            // Start the service AFTER .ready is written. Previously this
+            // was an AfterInstall hook on GVFS.Service.exe, but that races:
+            // the service's debounce timer could fire before .ready exists.
+            StagingUpdateService();
           end;
         MigrateConfigAndStatusCacheFiles();
         if (not KeepMountsRunning) and (ExpandConstant('{param:REMOUNTREPOS|true}') = 'true') then

GVFS/GVFS.Service/Handlers/RequestHandler.cs

@@ -29,6 +29,7 @@ public class RequestHandler
         private ITracer tracer;
         private IRepoRegistry repoRegistry;
         private Timer pendingUpgradeTimer;
+        private readonly object pendingUpgradeTimerLock = new object();
 
         public RequestHandler(ITracer tracer, string etwArea, IRepoRegistry repoRegistry)
         {
@@ -146,21 +147,24 @@ private void TryDeferredPendingUpgradeCheck(ITracer tracer)
             // Debounce: reset the timer on each unmount so the check fires
             // once after the last unmount settles. If multiple repos unmount
             // in quick succession, only one upgrade attempt runs.
-            if (this.pendingUpgradeTimer == null)
+            lock (this.pendingUpgradeTimerLock)
             {
-                this.pendingUpgradeTimer = new Timer(
-                    _ =>
-                    {
-                        tracer.RelatedInfo("TryDeferredPendingUpgradeCheck: Checking pending upgrade after unmount");
-                        PendingUpgradeHandler.TryApplyPendingUpgrade(tracer);
-                    },
-                    null,
-                    PendingUpgradeDelayMs,
-                    Timeout.Infinite);
-            }
-            else
-            {
-                this.pendingUpgradeTimer.Change(PendingUpgradeDelayMs, Timeout.Infinite);
+                if (this.pendingUpgradeTimer == null)
+                {
+                    this.pendingUpgradeTimer = new Timer(
+                        _ =>
+                        {
+                            tracer.RelatedInfo("TryDeferredPendingUpgradeCheck: Checking pending upgrade after unmount");
+                            PendingUpgradeHandler.TryApplyPendingUpgrade(tracer);
+                        },
+                        null,
+                        PendingUpgradeDelayMs,
+                        Timeout.Infinite);
+                }
+                else
+                {
+                    this.pendingUpgradeTimer.Change(PendingUpgradeDelayMs, Timeout.Infinite);
+                }
             }
         }
     }