Kill Background Process Trees

GVFS/GVFS.Common/GVFSPlatform.cs

@@ -57,7 +57,10 @@ namespace GVFS.Common
 
         public abstract Dictionary<string, string> GetPhysicalDiskInfo(string path, bool sizeStatsOnly);
 
-        public abstract bool IsConsoleOutputRedirectedToFile();
+        public abstract bool IsConsoleOutputRedirectedToFile();
+
+        public abstract bool TryKillProcessTree(int processId, out int exitCode, out string error);
+
         public abstract bool TryGetGVFSEnlistmentRoot(string directory, out string enlistmentRoot, out string errorMessage);
 
         public abstract bool IsGitStatusCacheSupported();

GVFS/GVFS.Common/Git/GitProcess.cs

@@ -126,34 +126,33 @@ namespace GVFS.Common.Git
             return true;
         }
 
-        public bool WaitForRunningGitProcess()
+        /// <summary>
+        /// Tries to kill the run git process.  Make sure you only use this on git processes that can safely be killed!
+        /// </summary>
+        /// <param name="processName">Name of the running process</param>
+        /// <param name="exitCode">Exit code of the kill.  -1 means there was no running process.</param>
+        /// <param name="error">Error message of the kill</param>
+        /// <returns></returns>
+        public bool TryKillRunningProcess(out string processName, out int exitCode, out string error)
         {
             this.stopping = true;
+            processName = null;
+            exitCode = -1;
+            error = null;
 
-            try
+            lock (this.processLock)
             {
-                lock (this.processLock)
+                Process process = this.executingProcess;
+
+                if (process != null)
                 {
-                    Process process = this.executingProcess;
+                    processName = process.ProcessName;
 
-                    if (process != null)
-                    {
-                        process.WaitForExit();
-                    }
-
-                    return true;
+                    return GVFSPlatform.Instance.TryKillProcessTree(process.Id, out exitCode, out error);
                 }
-            }
-            catch (Win32Exception)
-            {
-                // Thrown when process is already terminating
-            }
-            catch (InvalidOperationException)
-            {
-                // Process already terminated
-            }
 
-            return false;
+                return true;
+            }
         }
 
         public virtual void RevokeCredential(string repoUrl)

GVFS/GVFS.Common/InstallerPreRunChecker.cs

@@ -74,12 +74,10 @@ namespace GVFS.Upgrader
             return true;
         }
 
-        public virtual bool NoBlockingProcessCheck(out string consoleError)
+        public virtual bool IsInstallationBlockedByRunningProcess(out string consoleError)
         {
             consoleError = null;
 
-            this.tracer.RelatedInfo("Checking for running git processes.");
-
             // While checking for blocking processes like GVFS.Mount immediately after un-mounting,
             // then sometimes GVFS.Mount shows up as running. But if the check is done after waiting
             // for some time, then eventually GVFS.Mount goes away. The retry loop below is to help
@@ -87,7 +85,7 @@ namespace GVFS.Upgrader
             // actually quits.
             this.tracer.RelatedInfo("Checking if GVFS or dependent processes are running.");
             int retryCount = 10;
-            List<string> processList = null;
+            HashSet<string> processList = null;
             while (retryCount > 0)
             {
                 if (!this.IsBlockingProcessRunning(out processList))
@@ -105,7 +103,7 @@ namespace GVFS.Upgrader
                     Environment.NewLine,
                     "Blocking processes are running.",
                     $"Run {this.CommandToRerun} again after quitting these processes - " + string.Join(", ", processList.ToArray()));
-                this.tracer.RelatedError($"{nameof(this.TryUnmountAllGVFSRepos)}: {consoleError}");
+                this.tracer.RelatedError($"{nameof(this.IsInstallationBlockedByRunningProcess)}: {consoleError}");
                 return false;
             }
 
@@ -134,11 +132,11 @@ namespace GVFS.Upgrader
             return GVFSEnlistment.IsUnattended(this.tracer);
         }
 
-        protected virtual bool IsBlockingProcessRunning(out List<string> processes)
+        protected virtual bool IsBlockingProcessRunning(out HashSet<string> processes)
         {
             int currentProcessId = Process.GetCurrentProcess().Id;
             Process[] allProcesses = Process.GetProcesses();
-            List<string> matchingNames = new List<string>();
+            HashSet<string> matchingNames = new HashSet<string>();
 
             foreach (Process process in allProcesses)
             {

GVFS/GVFS.Common/Maintenance/GitMaintenanceStep.cs

@@ -79,18 +79,28 @@ namespace GVFS.Common.Maintenance
 
                 if (process != null)
                 {
-                    if (process.WaitForRunningGitProcess())
+                    if (process.TryKillRunningProcess(out string processName, out int exitCode, out string error))
                     {
                         this.Context.Tracer.RelatedEvent(
                             EventLevel.Informational,
-                            this.Area + ": waited for background Git process during " + nameof(this.Stop),
+                            string.Format(
+                                "{0}: killed background process {1} during {2}",
+                                this.Area,
+                                processName,
+                                nameof(this.Stop)),
                             metadata: null);
                     }
                     else
                     {
                         this.Context.Tracer.RelatedEvent(
                             EventLevel.Informational,
-                            this.Area + ": failed to wait for background Git process during " + nameof(this.Stop),
+                            string.Format(
+                                "{0}: failed to kill background process {1} during {2}. ExitCode:{3} Error:{4}",
+                                this.Area,
+                                processName,
+                                nameof(this.Stop),
+                                exitCode,
+                                error),
                             metadata: null);
                     }
                 }

GVFS/GVFS.Platform.Mac/MacPlatform.cs

@@ -159,5 +159,13 @@ namespace GVFS.Platform.Mac
         {
             return new MacFileBasedLock(fileSystem, tracer, lockPath);
         }
+
+        public override bool TryKillProcessTree(int processId, out int exitCode, out string error)
+        {
+            ProcessResult result = ProcessHelper.Run("pkill", $"-P {processId}");
+            error = result.Errors;
+            exitCode = result.ExitCode;
+            return result.ExitCode == 0;
+        }
     }
 }

GVFS/GVFS.Platform.Windows/WindowsPlatform.cs

@@ -332,6 +332,14 @@ namespace GVFS.Platform.Windows
             return WindowsPlatform.TryGetGVFSEnlistmentRootImplementation(directory, out enlistmentRoot, out errorMessage);
         }
 
+        public override bool TryKillProcessTree(int processId, out int exitCode, out string error)
+        {
+            ProcessResult result = ProcessHelper.Run("taskkill", $"/pid {processId} /f /t");
+            error = result.Errors;
+            exitCode = result.ExitCode;
+            return result.ExitCode == 0;
+        }
+
         private static object GetValueFromRegistry(RegistryHive registryHive, string key, string valueName, RegistryView view)
         {
             RegistryKey localKey = RegistryKey.OpenBaseKey(registryHive, view);

GVFS/GVFS.UnitTests.Windows/Windows/Mock/MockInstallerPreRunChecker.cs

@@ -72,9 +72,9 @@ namespace GVFS.UnitTests.Windows.Mock.Upgrader
             return this.FakedResultOfCheck(FailOnCheckType.UnattendedMode);
         }
 
-        protected override bool IsBlockingProcessRunning(out List<string> processes)
+        protected override bool IsBlockingProcessRunning(out HashSet<string> processes)
         {
-            processes = new List<string>();
+            processes = new HashSet<string>();
 
             bool isRunning = this.FakedResultOfCheck(FailOnCheckType.BlockingProcessesRunning);
             if (isRunning)

GVFS/GVFS.UnitTests/Git/GitProcessTests.cs

@@ -12,7 +12,11 @@ namespace GVFS.UnitTests.Git
         public void TryKillRunningProcess_NeverRan()
         {
             GitProcess process = new GitProcess(new MockGVFSEnlistment());
-            process.WaitForRunningGitProcess().ShouldBeTrue();
+            process.TryKillRunningProcess(out string processName, out int exitCode, out string error).ShouldBeTrue();
+
+            processName.ShouldBeNull();
+            exitCode.ShouldEqual(-1);
+            error.ShouldBeNull();
         }
 
         [TestCase]

GVFS/GVFS.UnitTests/Mock/Common/MockPlatform.cs

@@ -122,5 +122,12 @@ namespace GVFS.UnitTests.Mock.Common
         {
             return new MockFileBasedLock(fileSystem, tracer, lockPath);
         }
+
+        public override bool TryKillProcessTree(int processId, out int exitCode, out string error)
+        {
+            error = null;
+            exitCode = 0;
+            return true;
+        }
     }
 }

GVFS/GVFS.Upgrader/UpgradeOrchestrator.cs

@@ -222,18 +222,14 @@ namespace GVFS.Upgrader
             if (!this.LaunchInsideSpinner(
                 () =>
                 {
-                    // if (!this.preRunChecker.TryUnmountAllGVFSRepos(out error))
-
-                    if (!this.preRunChecker.NoBlockingProcessCheck(out error))
+                    if (!this.preRunChecker.IsInstallationBlockedByRunningProcess(out error))
                     {
                         return false;
                     }
 
-                    this.mount = true;
-
                     return true;
                 },
-                "Checking for running git processes."))
+                "Checking for blocking processes."))
             {
                 newVersion = null;
                 consoleError = error;