Skip to content

Commit bef5a36

Browse files
MuyuanMSCopilot
andcommitted
Address review: handle process-exit race and check elevation result
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8107e05 commit bef5a36

4 files changed

Lines changed: 29 additions & 8 deletions

File tree

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WindowWalker/Commands/EndTaskCommand.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ private static bool KillProcess(Window window)
6666
{
6767
window.Process.KillThisProcess(SettingsManager.Instance.KillProcessTree);
6868
}
69+
catch (ArgumentException)
70+
{
71+
// The process already exited between the existence check and the kill attempt.
72+
}
6973
catch (Win32Exception ex)
7074
{
7175
ExtensionHost.LogMessage(new LogMessage { Message = $"Failed to kill process '{window.Process.Name}' ({window.Process.ProcessID}) of the window '{window.Title}' ({window.Hwnd}): {ex.Message}" });

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WindowWalker/Components/WindowProcess.cs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The Microsoft Corporation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using System.ComponentModel;
67
using System.Diagnostics;
78
using Microsoft.CmdPal.Ext.WindowWalker.Commands;
@@ -220,10 +221,14 @@ internal static string GetProcessNameFromProcessID(uint pid)
220221
/// <param name="killProcessTree">Kill process and sub processes.</param>
221222
internal void KillThisProcess(bool killProcessTree)
222223
{
224+
var killTree = killProcessTree ? " /t" : string.Empty;
225+
223226
if (IsFullAccessDenied)
224227
{
225-
var killTree = killProcessTree ? " /t" : string.Empty;
226-
ExplorerInfoResultCommand.OpenInShell("taskkill.exe", $"/pid {(int)ProcessID} /f{killTree}", null, ExplorerInfoResultCommand.ShellRunAsType.Administrator, true);
228+
if (!ExplorerInfoResultCommand.OpenInShell("taskkill.exe", $"/pid {(int)ProcessID} /f{killTree}", null, ExplorerInfoResultCommand.ShellRunAsType.Administrator, true))
229+
{
230+
throw new InvalidOperationException($"Failed to start elevated taskkill for process ID {ProcessID}.");
231+
}
227232
}
228233
else
229234
{
@@ -236,8 +241,10 @@ internal void KillThisProcess(bool killProcessTree)
236241
{
237242
// Error 5 = ERROR_ACCESS_DENIED
238243
// Fall back to elevated taskkill when direct kill is denied
239-
var killTree = killProcessTree ? " /t" : string.Empty;
240-
ExplorerInfoResultCommand.OpenInShell("taskkill.exe", $"/pid {(int)ProcessID} /f{killTree}", null, ExplorerInfoResultCommand.ShellRunAsType.Administrator, true);
244+
if (!ExplorerInfoResultCommand.OpenInShell("taskkill.exe", $"/pid {(int)ProcessID} /f{killTree}", null, ExplorerInfoResultCommand.ShellRunAsType.Administrator, true))
245+
{
246+
throw new InvalidOperationException($"Failed to start elevated taskkill for process ID {ProcessID}.", ex);
247+
}
241248
}
242249
}
243250
}

src/modules/launcher/Plugins/Microsoft.Plugin.WindowWalker/Components/ContextMenuHelper.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ private static bool KillProcessCommand(Window window)
107107
{
108108
window.Process.KillThisProcess(WindowWalkerSettings.Instance.KillProcessTree);
109109
}
110+
catch (ArgumentException)
111+
{
112+
// The process already exited between the existence check and the kill attempt.
113+
}
110114
catch (Win32Exception ex)
111115
{
112116
Log.Exception($"Failed to kill process '{window.Process.Name}' ({window.Process.ProcessID}) of the window '{window.Title}' ({window.Hwnd}).", ex, typeof(ContextMenuHelper));

src/modules/launcher/Plugins/Microsoft.Plugin.WindowWalker/Components/WindowProcess.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,14 @@ internal static string GetProcessNameFromProcessID(uint pid)
209209
/// <param name="killProcessTree">Kill process and sub processes.</param>
210210
internal void KillThisProcess(bool killProcessTree)
211211
{
212+
string killTree = killProcessTree ? " /t" : string.Empty;
213+
212214
if (IsFullAccessDenied)
213215
{
214-
string killTree = killProcessTree ? " /t" : string.Empty;
215-
Helper.OpenInShell("taskkill.exe", $"/pid {(int)ProcessID} /f{killTree}", null, Helper.ShellRunAsType.Administrator, true);
216+
if (!Helper.OpenInShell("taskkill.exe", $"/pid {(int)ProcessID} /f{killTree}", null, Helper.ShellRunAsType.Administrator, true))
217+
{
218+
throw new InvalidOperationException($"Failed to start elevated taskkill for process ID {ProcessID}.");
219+
}
216220
}
217221
else
218222
{
@@ -225,8 +229,10 @@ internal void KillThisProcess(bool killProcessTree)
225229
{
226230
// Error 5 = ERROR_ACCESS_DENIED
227231
// Fall back to elevated taskkill when direct kill is denied
228-
string killTree = killProcessTree ? " /t" : string.Empty;
229-
Helper.OpenInShell("taskkill.exe", $"/pid {(int)ProcessID} /f{killTree}", null, Helper.ShellRunAsType.Administrator, true);
232+
if (!Helper.OpenInShell("taskkill.exe", $"/pid {(int)ProcessID} /f{killTree}", null, Helper.ShellRunAsType.Administrator, true))
233+
{
234+
throw new InvalidOperationException($"Failed to start elevated taskkill for process ID {ProcessID}.", ex);
235+
}
230236
}
231237
}
232238
}

0 commit comments

Comments
 (0)