Skip to content

Commit 44eb20d

Browse files
SteveL-MSFTmirichmo
SteveL-MSFT
authored and
mirichmo
committed
Fixes PowerShell#2534 by replacing expensive WMI query with Win32 API calls (PowerShell#2535)
* Fixes PowerShell#2534 by replacing expensive WMI query with Win32 API calls * fix break on unix build * added tests for PowerShell#2535 * although test passed, fixing exception that shows up * fixed Describe text * addressing code review feedback * addressing review feedback to comment on why sleep is needed added check that test processes are created before we try to kill them * fixed test to timeout and pending fix for PowerShell#2561
1 parent 0e8c809 commit 44eb20d

8 files changed

Lines changed: 261 additions & 45 deletions

File tree

‎build.psm1‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -597,13 +597,19 @@ function Publish-PSTestTools {
597597

598598
Find-Dotnet
599599

600+
$tools = "$PSScriptRoot/test/tools/EchoArgs","$PSScriptRoot/test/tools/CreateChildProcess"
600601
# Publish EchoArgs so it can be run by tests
601-
Push-Location "$PSScriptRoot/test/tools/EchoArgs"
602-
try {
603-
dotnet publish --output bin
604-
} finally {
605-
Pop-Location
602+
603+
foreach ($tool in $tools)
604+
{
605+
Push-Location $tool
606+
try {
607+
dotnet publish --output bin
608+
} finally {
609+
Pop-Location
610+
}
606611
}
612+
607613
}
608614

609615
function Start-PSPester {

‎src/System.Management.Automation/CoreCLR/CorePsStub.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,31 @@ public override bool IsInvalid
502502
}
503503
}
504504

505+
/// <summary>
506+
/// Stub for SafeHandleMinusOneIsInvalid
507+
/// </summary>
508+
public abstract class SafeHandleMinusOneIsInvalid : SafeHandle
509+
{
510+
/// <summary>
511+
/// Constructor
512+
/// </summary>
513+
protected SafeHandleMinusOneIsInvalid(bool ownsHandle)
514+
: base(new IntPtr(-1), ownsHandle)
515+
{
516+
}
517+
518+
/// <summary>
519+
/// IsInvalid
520+
/// </summary>
521+
public override bool IsInvalid
522+
{
523+
get
524+
{
525+
return handle == new IntPtr(-1);
526+
}
527+
}
528+
}
529+
505530
#endregion SafeHandle_Related
506531

507532
#region Misc_Types

‎src/System.Management.Automation/utils/PInvokeDllNames.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ internal static class PinvokeDllNames
132132
internal const string ReadConsoleInputDllName = "api-ms-win-core-console-l1-1-0.dll"; /*117*/
133133
internal const string GetVersionExDllName = "api-ms-win-core-sysinfo-l1-1-0.dll"; /*118*/
134134
internal const string FormatMessageDllName = "api-ms-win-core-localization-l1-2-0.dll"; /*119*/
135+
internal const string CreateToolhelp32SnapshotDllName = "api-ms-win-core-toolhelp-l1-1-0"; /*120*/
136+
internal const string Process32FirstDllName = "api-ms-win-core-toolhelp-l1-1-0"; /*121*/
137+
internal const string Process32NextDllName = "api-ms-win-core-toolhelp-l1-1-0"; /*122*/
135138
#else
136139
internal const string QueryDosDeviceDllName = "kernel32.dll"; /*1*/
137140
internal const string CreateSymbolicLinkDllName = "kernel32.dll"; /*2*/
@@ -251,6 +254,9 @@ internal static class PinvokeDllNames
251254
internal const string ReadConsoleInputDllName = "kernel32.dll"; /*117*/
252255
internal const string GetVersionExDllName = "kernel32.dll"; /*118*/
253256
internal const string FormatMessageDllName = "wevtapi.dll"; /*119*/
257+
internal const string CreateToolhelp32SnapshotDllName = "kernel32.dll"; /*120*/
258+
internal const string Process32FirstDllName = "kernel32.dll"; /*121*/
259+
internal const string Process32NextDllName = "kernel32.dll"; /*122*/
254260
#endif
255261
}
256262
}

‎src/System.Management.Automation/utils/PlatformInvokes.cs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,68 @@ internal enum StandardHandleId : uint
736736
[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
737737
public static extern IntPtr GetStdHandle(uint handleId);
738738

739+
#endif
740+
741+
#endregion
742+
743+
#region CreateToolhelp32Snapshot
744+
745+
#if !UNIX
746+
747+
[DllImport(PinvokeDllNames.CreateToolhelp32SnapshotDllName, SetLastError = true)]
748+
internal static extern SafeSnapshotHandle CreateToolhelp32Snapshot(SnapshotFlags flags, uint id);
749+
[DllImport(PinvokeDllNames.Process32FirstDllName, SetLastError = true)]
750+
internal static extern bool Process32First(SafeSnapshotHandle hSnapshot, ref PROCESSENTRY32 lppe);
751+
[DllImport(PinvokeDllNames.Process32NextDllName, SetLastError = true)]
752+
internal static extern bool Process32Next(SafeSnapshotHandle hSnapshot, ref PROCESSENTRY32 lppe);
753+
754+
internal sealed class SafeSnapshotHandle : SafeHandleMinusOneIsInvalid
755+
{
756+
internal SafeSnapshotHandle() : base(true)
757+
{
758+
}
759+
760+
[SecurityPermission(SecurityAction.LinkDemand, UnmanagedCode = true)]
761+
internal SafeSnapshotHandle(IntPtr handle) : base(true)
762+
{
763+
base.SetHandle(handle);
764+
}
765+
766+
protected override bool ReleaseHandle()
767+
{
768+
return CloseHandle(base.handle);
769+
}
770+
}
771+
772+
[Flags]
773+
internal enum SnapshotFlags : uint
774+
{
775+
HeapList = 0x00000001,
776+
Process = 0x00000002,
777+
Thread = 0x00000004,
778+
Module = 0x00000008,
779+
Module32 = 0x00000010,
780+
All = (HeapList | Process | Thread | Module),
781+
Inherit = 0x80000000,
782+
NoHeaps = 0x40000000
783+
}
784+
[StructLayout(LayoutKind.Sequential)]
785+
internal struct PROCESSENTRY32
786+
{
787+
public uint dwSize;
788+
public uint cntUsage;
789+
public uint th32ProcessID;
790+
public IntPtr th32DefaultHeapID;
791+
public uint th32ModuleID;
792+
public uint cntThreads;
793+
public uint th32ParentProcessID;
794+
public int pcPriClassBase;
795+
public uint dwFlags;
796+
[MarshalAs(UnmanagedType.ByValTStr, SizeConst = 260)] public string szExeFile;
797+
};
798+
799+
internal const int ERROR_NO_MORE_FILES = 0x12;
800+
739801
#endif
740802

741803
#endregion

‎src/System.Management.Automation/utils/PsUtils.cs‎

Lines changed: 49 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
using System.Collections;
66
using System.Diagnostics;
7-
using System.Globalization;
87
using System.IO;
98
using System.Linq;
109
using System.Reflection;
@@ -16,7 +15,10 @@
1615
using Microsoft.Win32;
1716
using System.Collections.Generic;
1817
using System.Management.Automation.Language;
19-
using Microsoft.Management.Infrastructure;
18+
#if CORECLR
19+
// Use stubs for SerializableAttribute, SecurityPermissionAttribute, ReliabilityContractAttribute and ISerializable related types.
20+
using Microsoft.PowerShell.CoreClr.Stubs;
21+
#endif
2022

2123
namespace System.Management.Automation
2224
{
@@ -91,57 +93,64 @@ internal static ProcessModule GetMainModule(Process targetProcess)
9193
/// <summary>
9294
/// Retrieve the parent process of a process.
9395
///
94-
/// This is an extremely expensive operation, as WMI
95-
/// needs to work with an ugly Win32 API. The Win32 API
96-
/// creates a snapshot of every process in the system, which
97-
/// you then need to iterate through to find your process and
98-
/// its parent PID.
99-
///
100-
/// Also, since this is PID based, this API is only reliable
101-
/// when the process has not yet exited.
96+
/// Previously this code used WMI, but WMI is causing a CPU spike whenever the query gets called as it results in
97+
/// tzres.dll and tzres.mui.dll being loaded into every process to conver the time information to local format.
98+
/// For perf reasons, we result to P/Invoke.
10299
/// </summary>
103100
///
104101
/// <param name="current">The process we want to find the
105102
/// parent of</param>
106103
internal static Process GetParentProcess(Process current)
107104
{
108-
string wmiQuery = String.Format(CultureInfo.CurrentCulture,
109-
"Select * From Win32_Process Where Handle='{0}'",
110-
current.Id);
111-
112-
using (CimSession cimSession = CimSession.Create(null))
113-
{
114-
IEnumerable<CimInstance> processCollection =
115-
cimSession.QueryInstances("root/cimv2", "WQL", wmiQuery);
116-
117-
int parentPid =
118-
processCollection.Select(
119-
cimProcess =>
120-
Convert.ToInt32(cimProcess.CimInstanceProperties["ParentProcessId"].Value,
121-
CultureInfo.CurrentCulture)).FirstOrDefault();
105+
int parentPid = 0;
122106

123-
if (parentPid == 0)
124-
return null;
107+
#if !UNIX
108+
PlatformInvokes.PROCESSENTRY32 pe32 = new PlatformInvokes.PROCESSENTRY32 { };
109+
pe32.dwSize = (uint)ClrFacade.SizeOf<PlatformInvokes.PROCESSENTRY32>();
125110

126-
try
111+
using (PlatformInvokes.SafeSnapshotHandle hSnapshot = PlatformInvokes.CreateToolhelp32Snapshot(PlatformInvokes.SnapshotFlags.Process, (uint)current.Id))
112+
{
113+
if (!PlatformInvokes.Process32First(hSnapshot, ref pe32))
127114
{
128-
Process returnProcess = Process.GetProcessById(parentPid);
129-
130-
// Ensure the process started before the current
131-
// process, as it could have gone away and had the
132-
// PID recycled.
133-
if (returnProcess.StartTime <= current.StartTime)
134-
return returnProcess;
135-
else
115+
int errno = Marshal.GetLastWin32Error();
116+
if (errno == PlatformInvokes.ERROR_NO_MORE_FILES)
117+
{
136118
return null;
119+
}
137120
}
138-
catch (ArgumentException)
121+
do
139122
{
140-
// GetProcessById throws an ArgumentException when
141-
// you reach the top of the chain -- Explorer.exe
142-
// has a parent process, but you cannot retrieve it.
123+
if (pe32.th32ProcessID == (uint)current.Id)
124+
{
125+
parentPid = (int)pe32.th32ParentProcessID;
126+
break;
127+
}
128+
129+
} while (PlatformInvokes.Process32Next(hSnapshot, ref pe32));
130+
}
131+
#endif
132+
133+
if (parentPid == 0)
134+
return null;
135+
136+
try
137+
{
138+
Process returnProcess = Process.GetProcessById(parentPid);
139+
140+
// Ensure the process started before the current
141+
// process, as it could have gone away and had the
142+
// PID recycled.
143+
if (returnProcess.StartTime <= current.StartTime)
144+
return returnProcess;
145+
else
143146
return null;
144-
}
147+
}
148+
catch (ArgumentException)
149+
{
150+
// GetProcessById throws an ArgumentException when
151+
// you reach the top of the chain -- Explorer.exe
152+
// has a parent process, but you cannot retrieve it.
153+
return null;
145154
}
146155
}
147156

‎test/powershell/Language/Scripting/NativeExecution/NativeCommandProcessor.Tests.ps1‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
Describe "Native Command Processor" -tags "Feature" {
2+
3+
BeforeAll {
4+
# Find where test/powershell is so we can find the createchildprocess command relative to it
5+
$powershellTestDir = $PSScriptRoot
6+
while ($powershellTestDir -notmatch 'test[\\/]powershell$') {
7+
$powershellTestDir = Split-Path $powershellTestDir
8+
}
9+
$createchildprocess = Join-Path (Split-Path $powershellTestDir) tools/CreateChildProcess/bin/createchildprocess
10+
}
11+
12+
# If powershell receives a StopProcessing, it should kill the native process and all child processes
13+
14+
# this test should pass and no longer Penidng when #2561 is fixed
15+
It "Should kill native process tree" {
16+
17+
Test-Path $createchildprocess | Should Be $true
18+
19+
# make sure no test processes are running
20+
# on Linux, the Process class truncates the name so filter using Where-Object
21+
Get-Process | Where-Object {$_.Name -like 'createchildproc'} | Stop-Process
22+
23+
[int] $numToCreate = 2
24+
25+
$ps = [PowerShell]::Create().AddCommand($createchildprocess)
26+
$ps.AddParameter($numToCreate)
27+
$async = $ps.BeginInvoke()
28+
$ps.InvocationStateInfo.State | Should Be "Running"
29+
30+
[bool] $childrenCreated = $false
31+
while (-not $childrenCreated)
32+
{
33+
$childprocesses = Get-Process | Where-Object {$_.Name -like 'createchildproc'}
34+
if ($childprocesses.count -eq $numToCreate+1)
35+
{
36+
$childrenCreated = $true
37+
}
38+
}
39+
40+
$startTime = Get-Date
41+
$beginsync = $ps.BeginStop($null, $async)
42+
# wait no more than 5 secs for the processes to be terminated, otherwise test has failed
43+
while (((Get-Date) - $startTime).TotalSeconds -lt 5)
44+
{
45+
if (($childprocesses.hasexited -eq $true).count -eq $numToCreate+1)
46+
{
47+
break
48+
}
49+
}
50+
$childprocesses = Get-Process | Where-Object {$_.Name -like 'createchildproc'}
51+
$count = $childprocesses.count
52+
$childprocesses | Stop-Process
53+
$count | Should Be 0
54+
}
55+
}

‎test/tools/CreateChildProcess/CreateChildProcess.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
using System;
2+
using System.Diagnostics;
3+
using System.Threading;
4+
5+
namespace CreateChildProcess
6+
{
7+
class Program
8+
{
9+
static void Main(string[] args)
10+
{
11+
if (args.Length > 0)
12+
{
13+
uint num = UInt32.Parse(args[0]);
14+
for (uint i = 0; i < num; i++)
15+
{
16+
Process child = new Process();
17+
child.StartInfo.FileName = Process.GetCurrentProcess().MainModule.FileName;
18+
child.Start();
19+
}
20+
}
21+
// sleep is needed so the process doesn't exit before the test case kill it
22+
Thread.Sleep(100000);
23+
}
24+
}
25+
}

‎test/tools/CreateChildProcess/project.json‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
"name": "createchildprocess",
3+
"version": "1.0.0-*",
4+
"description": "Very simple little console app that creates child processes of itself",
5+
6+
"buildOptions": {
7+
"emitEntryPoint": true
8+
},
9+
10+
"frameworks": {
11+
"netcoreapp1.0": {
12+
"dependencies": {
13+
"Microsoft.NETCore.App": "1.1.0-preview1-001100-00"
14+
}
15+
}
16+
},
17+
18+
"runtimes": {
19+
"ubuntu.16.04-x64": { },
20+
"ubuntu.14.04-x64": { },
21+
"debian.8-x64": { },
22+
"centos.7-x64": { },
23+
"win7-x64": { },
24+
"win81-x64": { },
25+
"win10-x64": { },
26+
"osx.10.11-x64": { }
27+
}
28+
}

0 commit comments

Comments
 (0)