From 35a3378164aac7961b13f357b06f7eef2db094db Mon Sep 17 00:00:00 2001 From: iSazonov Date: Wed, 17 Jan 2018 13:02:11 +0300 Subject: [PATCH 1/3] Set local pipeline thread stack size --- .../engine/PSConfiguration.cs | 11 +++- .../engine/Utils.cs | 25 +++++++- .../engine/hostifaces/LocalPipeline.cs | 60 +++++++------------ 3 files changed, 53 insertions(+), 43 deletions(-) diff --git a/src/System.Management.Automation/engine/PSConfiguration.cs b/src/System.Management.Automation/engine/PSConfiguration.cs index ef01eba36c7..115e58a01a5 100644 --- a/src/System.Management.Automation/engine/PSConfiguration.cs +++ b/src/System.Management.Automation/engine/PSConfiguration.cs @@ -331,7 +331,7 @@ internal PSKeyword GetLogKeywords() return result; } #endif // UNIX - + private T ReadValueFromFile(string fileName, string key, T defaultValue = default(T), Func readImpl = null) { @@ -548,6 +548,7 @@ internal sealed class PowerShellPolicies public ProtectedEventLogging ProtectedEventLogging { get; set; } public Transcription Transcription { get; set; } public UpdatableHelp UpdatableHelp { get; set; } + public UpdatableHelp PipelineMaxStackSizeMB { get; set; } public ConsoleSessionConfiguration ConsoleSessionConfiguration { get; set; } } @@ -608,6 +609,14 @@ internal sealed class ConsoleSessionConfiguration : PolicyBase public string ConsoleSessionConfigurationName { get; set; } } + /// + /// Setting about PipelineMaxStackSize + /// + internal sealed class PipelineMaxStackSize : PolicyBase + { + public int PipelineMaxStackSizeMB { get; set; } + } + /// /// Setting about ProtectedEventLogging /// diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 3659d0c4cdd..2d26c65cc7e 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -507,7 +507,7 @@ internal static bool IsValidPSEditionValue(string editionValue) T policy = null; #if !UNIX // On Windows, group policy settings from registry take precedence. - // If the requested policy is not defined in registry, we query the configuration file. + // If the requested policy is not defined in registry, we query the configuration file. policy = GetPolicySettingFromGPO(preferenceOrder); if (policy != null) { return policy; } #endif @@ -547,6 +547,7 @@ internal static bool IsValidPSEditionValue(string editionValue) case nameof(ScriptExecution): result = policies.ScriptExecution; break; case nameof(ScriptBlockLogging): result = policies.ScriptBlockLogging; break; case nameof(ModuleLogging): result = policies.ModuleLogging; break; + case nameof(PipelineMaxStackSize): result = policies.PipelineMaxStackSizeMB; break; case nameof(ProtectedEventLogging): result = policies.ProtectedEventLogging; break; case nameof(Transcription): result = policies.Transcription; break; case nameof(UpdatableHelp): result = policies.UpdatableHelp; break; @@ -566,6 +567,7 @@ internal static bool IsValidPSEditionValue(string editionValue) {nameof(ScriptExecution), @"Software\Policies\Microsoft\PowerShellCore"}, {nameof(ScriptBlockLogging), @"Software\Policies\Microsoft\PowerShellCore\ScriptBlockLogging"}, {nameof(ModuleLogging), @"Software\Policies\Microsoft\PowerShellCore\ModuleLogging"}, + {nameof(PipelineMaxStackSize), @"Software\Policies\Microsoft\PowerShellCore\PipelineMaxStackSize"}, {nameof(ProtectedEventLogging), @"Software\Policies\Microsoft\Windows\EventLog\ProtectedEventLogging"}, {nameof(Transcription), @"Software\Policies\Microsoft\PowerShellCore\Transcription"}, {nameof(UpdatableHelp), @"Software\Policies\Microsoft\PowerShellCore\UpdatableHelp"}, @@ -586,8 +588,11 @@ internal static bool IsValidPSEditionValue(string editionValue) GroupPolicyKeys.TryGetValue(tType.Name, out string gpoKeyPath); Diagnostics.Assert(gpoKeyPath != null, StringUtil.Format("The GPO registry key path should be pre-defined for {0}", tType.Name)); - using (RegistryKey gpoKey = rootKey.OpenSubKey(gpoKeyPath)) + RegistryKey gpoKey = null; + try { + gpoKey = rootKey.OpenSubKey(gpoKeyPath, writable: false); + // If the corresponding GPO key doesn't exist, return null if (gpoKey == null) { return null; } @@ -614,7 +619,7 @@ internal static bool IsValidPSEditionValue(string editionValue) } else if (subKeyNameSet != null && subKeyNameSet.Contains(settingName)) { - using (RegistryKey subKey = gpoKey.OpenSubKey(settingName)) + using (RegistryKey subKey = gpoKey.OpenSubKey(settingName, writable: false)) { if (subKey != null) { rawRegistryValue = subKey.GetValueNames(); } } @@ -669,6 +674,20 @@ internal static bool IsValidPSEditionValue(string editionValue) // If no property is set, then we consider this policy as undefined return isAnyPropertySet ? (T) tInstance : null; } + catch + { + // In any case, if the registry key or value does not exist or we do not have permissions to read it, + // we continue silently to use a default value. + } + finally + { + if (gpoKey != null) + { + gpoKey.Dispose(); + } + } + + return null; } /// diff --git a/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs b/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs index 2cac96bb84f..e5085994e50 100644 --- a/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs +++ b/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs @@ -7,6 +7,7 @@ using System.Globalization; using System.Threading; using Microsoft.Win32; +using System.Management.Automation.Configuration; using System.Management.Automation.Internal; using System.Management.Automation.Internal.Host; using System.Management.Automation.Tracing; @@ -247,54 +248,35 @@ private void SetupInvokeThread(Thread invokeThread, bool changeName) } } -#if !CORECLR /// - /// Stack Reserve setting for pipeline threads + /// Set stack size for local pipeline threads. + /// Default is 10MB. + /// Default 10MB may be too large if we use many local pipelines for paralleling.PipelineMaxStackSizeMB /// - internal static int MaxStack - { - get - { - int i = ReadRegistryInt("PipelineMaxStackSizeMB", 10); - if (i < 10) - i = 10; // minimum 10MB - else if (i > 100) - i = 100; // maximum 100MB - return i * 1000000; - } - } + internal static int StackSize => GetPipelineMaxStackSizeOption(10); - internal static int ReadRegistryInt(string policyValueName, int defaultValue) + private static int GetPipelineMaxStackSizeOption(int defaultValue) { - RegistryKey key; - try - { - key = Registry.LocalMachine.OpenSubKey("SOFTWARE\\Microsoft\\PowerShell\\1\\ShellIds"); - } - catch (System.Security.SecurityException) - { - return defaultValue; - } - if (null == key) - return defaultValue; + int stackSize = defaultValue; - object temp; - try + var pipelineMaxStackSizeOption = Utils.GetPolicySetting(Utils.SystemWideThenCurrentUserConfig); + + if (pipelineMaxStackSizeOption != null) { - temp = key.GetValue(policyValueName); + stackSize = pipelineMaxStackSizeOption.PipelineMaxStackSizeMB; } - catch (System.Security.SecurityException) + + if (stackSize < defaultValue) { - return defaultValue; + stackSize = defaultValue; // minimum 10MB } - if (!(temp is int)) + else if (stackSize > 100) { - return defaultValue; + stackSize = 100; // maximum 100MB } - int i = (int)temp; - return i; + + return stackSize * 1_000_000; } -#endif /// /// Helper method for asynchronous invoke @@ -1225,17 +1207,17 @@ protected override } /// - /// Helper class that holds the thread used to execute pipelines when CreateThreadOptions.ReuseThread is used + /// Helper class that holds the thread used to execute pipelines when CreateThreadOptions.ReuseThread is used. /// internal class PipelineThread : IDisposable { /// - /// Creates the worker thread and waits for it to be ready + /// Creates the worker thread and waits for it to be ready. /// #if CORECLR internal PipelineThread() { - _worker = new Thread(WorkerProc); + _worker = new Thread(WorkerProc, LocalPipeline.StackSize); _workItem = null; _workItemReady = new AutoResetEvent(false); _closed = false; From d229ae8964d2b0e1994fe1b4f11444f34bbecb01 Mon Sep 17 00:00:00 2001 From: iSazonov Date: Thu, 18 Jan 2018 11:55:46 +0300 Subject: [PATCH 2/3] Remove StackSize property, change Registry key, fix typo --- .../engine/PSConfiguration.cs | 2 +- .../engine/Utils.cs | 2 +- .../engine/hostifaces/LocalPipeline.cs | 19 ++++++++++--------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/System.Management.Automation/engine/PSConfiguration.cs b/src/System.Management.Automation/engine/PSConfiguration.cs index 115e58a01a5..cc767b8d74c 100644 --- a/src/System.Management.Automation/engine/PSConfiguration.cs +++ b/src/System.Management.Automation/engine/PSConfiguration.cs @@ -548,7 +548,7 @@ internal sealed class PowerShellPolicies public ProtectedEventLogging ProtectedEventLogging { get; set; } public Transcription Transcription { get; set; } public UpdatableHelp UpdatableHelp { get; set; } - public UpdatableHelp PipelineMaxStackSizeMB { get; set; } + public PipelineMaxStackSize PipelineMaxStackSizeMB { get; set; } public ConsoleSessionConfiguration ConsoleSessionConfiguration { get; set; } } diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 2d26c65cc7e..5a16d17a173 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -567,7 +567,7 @@ internal static bool IsValidPSEditionValue(string editionValue) {nameof(ScriptExecution), @"Software\Policies\Microsoft\PowerShellCore"}, {nameof(ScriptBlockLogging), @"Software\Policies\Microsoft\PowerShellCore\ScriptBlockLogging"}, {nameof(ModuleLogging), @"Software\Policies\Microsoft\PowerShellCore\ModuleLogging"}, - {nameof(PipelineMaxStackSize), @"Software\Policies\Microsoft\PowerShellCore\PipelineMaxStackSize"}, + {nameof(PipelineMaxStackSize), @"Software\Policies\Microsoft\PowerShellCore"}, {nameof(ProtectedEventLogging), @"Software\Policies\Microsoft\Windows\EventLog\ProtectedEventLogging"}, {nameof(Transcription), @"Software\Policies\Microsoft\PowerShellCore\Transcription"}, {nameof(UpdatableHelp), @"Software\Policies\Microsoft\PowerShellCore\UpdatableHelp"}, diff --git a/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs b/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs index e5085994e50..c5af6b2e2a4 100644 --- a/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs +++ b/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs @@ -253,11 +253,12 @@ private void SetupInvokeThread(Thread invokeThread, bool changeName) /// Default is 10MB. /// Default 10MB may be too large if we use many local pipelines for paralleling.PipelineMaxStackSizeMB /// - internal static int StackSize => GetPipelineMaxStackSizeOption(10); + private const int DefaultPipelineStackSize = 10; + private const int MaxPipelineStackSize = 100; - private static int GetPipelineMaxStackSizeOption(int defaultValue) + internal static int GetPipelineStackSizeOption() { - int stackSize = defaultValue; + int stackSize = DefaultPipelineStackSize; var pipelineMaxStackSizeOption = Utils.GetPolicySetting(Utils.SystemWideThenCurrentUserConfig); @@ -266,13 +267,13 @@ private static int GetPipelineMaxStackSizeOption(int defaultValue) stackSize = pipelineMaxStackSizeOption.PipelineMaxStackSizeMB; } - if (stackSize < defaultValue) + if (stackSize < DefaultPipelineStackSize) { - stackSize = defaultValue; // minimum 10MB + stackSize = DefaultPipelineStackSize; } - else if (stackSize > 100) + else if (stackSize > MaxPipelineStackSize) { - stackSize = 100; // maximum 100MB + stackSize = MaxPipelineStackSize; } return stackSize * 1_000_000; @@ -1217,7 +1218,7 @@ internal class PipelineThread : IDisposable #if CORECLR internal PipelineThread() { - _worker = new Thread(WorkerProc, LocalPipeline.StackSize); + _worker = new Thread(WorkerProc, LocalPipeline.GetPipelineStackSizeOption()); _workItem = null; _workItemReady = new AutoResetEvent(false); _closed = false; @@ -1225,7 +1226,7 @@ internal PipelineThread() #else internal PipelineThread(ApartmentState apartmentState) { - _worker = new Thread(WorkerProc, LocalPipeline.MaxStack); + _worker = new Thread(WorkerProc, LocalPipeline.GetPipelineStackSizeOption()); _workItem = null; _workItemReady = new AutoResetEvent(false); _closed = false; From 79b91ae76ac7988fc058080dc786b5d70ebec64a Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 22 Feb 2018 01:15:31 +0500 Subject: [PATCH 3/3] Fix property assign --- src/System.Management.Automation/engine/Utils.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 5a16d17a173..9bc948e5cb7 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -634,11 +634,17 @@ internal static bool IsValidPSEditionValue(string editionValue) switch (propertyType) { - case var _ when propertyType == typeof(bool?): + case var _ when propertyType == typeof(int): if (rawRegistryValue is int rawIntValue) { - if (rawIntValue == 1) { propertyValue = true; } - else if (rawIntValue == 0) { propertyValue = false; } + propertyValue = rawIntValue; + } + break; + case var _ when propertyType == typeof(bool?): + if (rawRegistryValue is int rawBoolValue) + { + if (rawBoolValue == 1) { propertyValue = true; } + else if (rawBoolValue == 0) { propertyValue = false; } } break; case var _ when propertyType == typeof(string):