Skip to content

Add fix of process objects for processes that have exited. #16946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PaulHigin merged 4 commits into from
Mar 2, 2022
Merged

Add fix of process objects for processes that have exited. #16946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5437de
Add fix for process objects for processes that have exited.
PaulHigin Mar 1, 2022
569ec1e
Fix code factor
PaulHigin Mar 1, 2022
e35f4d8
Fix typo
PaulHigin Mar 1, 2022
32733c7
Add try/catch for possible race condition on exiting process.
PaulHigin Mar 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 68 additions & 23 deletions src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,22 +318,19 @@ private static void PrepareRunspace(Runspace runspace)

private Process GetProcessById(int procId)
{
try
{
return Process.GetProcessById(procId);
}
catch (System.ArgumentException)
var process = PSHostProcessUtils.GetProcessById(procId);
if (process is null)
{
ThrowTerminatingError(
new ErrorRecord(
new PSArgumentException(StringUtil.Format(RemotingErrorIdStrings.EnterPSHostProcessNoProcessFoundWithId, procId)),
"EnterPSHostProcessNoProcessFoundWithId",
ErrorCategory.InvalidArgument,
this)
);

return null;
new ErrorRecord(
new PSArgumentException(StringUtil.Format(RemotingErrorIdStrings.EnterPSHostProcessNoProcessFoundWithId, procId)),
"EnterPSHostProcessNoProcessFoundWithId",
ErrorCategory.InvalidArgument,
this)
);
}

return process;
}

private Process GetProcessByHostProcessInfo(PSHostProcessInfo hostProcessInfo)
Expand Down Expand Up @@ -403,7 +400,7 @@ private void VerifyProcess(Process process)
{
ThrowTerminatingError(
new ErrorRecord(
new PSInvalidOperationException(StringUtil.Format(RemotingErrorIdStrings.EnterPSHostProcessNoPowerShell, Process.ProcessName)),
new PSInvalidOperationException(StringUtil.Format(RemotingErrorIdStrings.EnterPSHostProcessNoPowerShell, Process.Id)),
"EnterPSHostProcessNoPowerShell",
ErrorCategory.InvalidOperation,
this)
Expand Down Expand Up @@ -599,9 +596,22 @@ private static int[] GetProcIdsFromNames(string[] names)
WildcardPattern namePattern = WildcardPattern.Get(name, WildcardOptions.IgnoreCase);
foreach (var proc in processes)
{
if (namePattern.IsMatch(proc.ProcessName))
// Skip processes that have already terminated.
if (proc.HasExited)
{
continue;
}

try
{
returnIds.Add(proc.Id);
if (namePattern.IsMatch(proc.ProcessName))
{
returnIds.Add(proc.Id);
}
}
catch (InvalidOperationException)
{
// Ignore if process has exited in the mean time.
}
}
}
Expand Down Expand Up @@ -681,10 +691,9 @@ internal static IReadOnlyCollection<PSHostProcessInfo> GetAppDomainNamesFromProc
string pName = namedPipe.Substring(pNameIndex + 1);

Process process = null;

try
{
process = System.Diagnostics.Process.GetProcessById(id);
process = PSHostProcessUtils.GetProcessById(id);
}
catch (Exception)
{
Expand All @@ -704,10 +713,20 @@ internal static IReadOnlyCollection<PSHostProcessInfo> GetAppDomainNamesFromProc
// best effort to cleanup
}
}
else if (process.ProcessName.Equals(pName, StringComparison.Ordinal))
else
{
// only add if the process name matches
procAppDomainInfo.Add(new PSHostProcessInfo(pName, id, appDomainName, namedPipe));
try
{
if (process.ProcessName.Equals(pName, StringComparison.Ordinal))
{
// only add if the process name matches
procAppDomainInfo.Add(new PSHostProcessInfo(pName, id, appDomainName, namedPipe));
}
}
catch (InvalidOperationException)
{
// Ignore if process has exited in the mean time.
}
}
}
}
Expand Down Expand Up @@ -796,8 +815,8 @@ internal PSHostProcessInfo(
MainWindowTitle = string.Empty;
try
{
var proc = Process.GetProcessById(processId);
MainWindowTitle = proc.MainWindowTitle ?? string.Empty;
var process = PSHostProcessUtils.GetProcessById(processId);
MainWindowTitle = process?.MainWindowTitle ?? string.Empty;
}
catch (ArgumentException)
{
Expand Down Expand Up @@ -831,4 +850,30 @@ public string GetPipeNameFilePath()
}

#endregion

#region PSHostProcessUtils

internal static class PSHostProcessUtils
{
/// <summary>
/// Return a System.Diagnostics.Process object by process Id,
/// or null if not found or process has exited.
/// </summary>
/// <param name="procId">Process of Id to find.</param>
/// <returns>Process object or null.</returns>
public static Process GetProcessById(int procId)
{
try
{
var process = Process.GetProcessById(procId);
return process.HasExited ? null : process;
}
catch (System.ArgumentException)
{
return null;
}
}
}

#endregion
}
2 changes: 1 addition & 1 deletion src/System.Management.Automation/resources/RemotingErrorIdStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1408,7 +1408,7 @@ All WinRM sessions connected to PowerShell session configurations, such as Micro
<value>Multiple processes were found with this name {0}. Use the process Id to specify a single process to enter.</value>
</data>
<data name="EnterPSHostProcessNoPowerShell" xml:space="preserve">
<value>Cannot enter process {0} because it has not loaded the PowerShell engine.</value>
<value>Cannot enter process with Id '{0}' because it has not loaded the PowerShell engine.</value>
</data>
<data name="EnterPSHostProcessNoProcessFoundWithId" xml:space="preserve">
<value>No process was found with Id: {0}.</value>
Expand Down