From 239afe27dc4a9500e688c0e9ccf0c22220855178 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 21 Sep 2019 12:10:03 +0500 Subject: [PATCH 1/5] Fix style issues in Utils.cs --- .../engine/Utils.cs | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index d20aebc88e8..c8557d91d7b 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -809,11 +809,11 @@ internal static bool IsValidPSEditionValue(string editionValue) private static readonly Dictionary WindowsPowershellGroupPolicyKeys = new Dictionary { - {nameof(ScriptExecution), @"Software\Policies\Microsoft\Windows\PowerShell"}, - {nameof(ScriptBlockLogging), @"Software\Policies\Microsoft\Windows\PowerShell\ScriptBlockLogging"}, - {nameof(ModuleLogging), @"Software\Policies\Microsoft\Windows\PowerShell\ModuleLogging"}, - {nameof(Transcription), @"Software\Policies\Microsoft\Windows\PowerShell\Transcription"}, - {nameof(UpdatableHelp), @"Software\Policies\Microsoft\Windows\PowerShell\UpdatableHelp"}, + { nameof(ScriptExecution), @"Software\Policies\Microsoft\Windows\PowerShell" }, + { nameof(ScriptBlockLogging), @"Software\Policies\Microsoft\Windows\PowerShell\ScriptBlockLogging" }, + { nameof(ModuleLogging), @"Software\Policies\Microsoft\Windows\PowerShell\ModuleLogging" }, + { nameof(Transcription), @"Software\Policies\Microsoft\Windows\PowerShell\Transcription" }, + { nameof(UpdatableHelp), @"Software\Policies\Microsoft\Windows\PowerShell\UpdatableHelp" }, }; private const string PolicySettingFallbackKey = "UseWindowsPowerShellPolicySetting"; @@ -858,7 +858,10 @@ private static bool TrySetPolicySettingsFromRegistryKey(object instance, Type in { using (RegistryKey subKey = gpoKey.OpenSubKey(settingName)) { - if (subKey != null) { rawRegistryValue = subKey.GetValueNames(); } + if (subKey != null) + { + rawRegistryValue = subKey.GetValueNames(); + } } } @@ -874,8 +877,14 @@ private static bool TrySetPolicySettingsFromRegistryKey(object instance, Type in case var _ when propertyType == typeof(bool?): if (rawRegistryValue is int rawIntValue) { - if (rawIntValue == 1) { propertyValue = true; } - else if (rawIntValue == 0) { propertyValue = false; } + if (rawIntValue == 1) + { + propertyValue = true; + } + else if (rawIntValue == 0) + { + propertyValue = false; + } } break; From ddd71f4ec4ad10187040f23f4c38c56fdb451896 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 21 Sep 2019 12:21:04 +0500 Subject: [PATCH 2/5] Fix some style issues --- .../remoting/server/OutOfProcServerMediator.cs | 10 +++++++--- .../remoting/server/ServerRunspacePoolDriver.cs | 15 +++++++-------- .../engine/remoting/server/serverremotesession.cs | 3 ++- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/System.Management.Automation/engine/remoting/server/OutOfProcServerMediator.cs b/src/System.Management.Automation/engine/remoting/server/OutOfProcServerMediator.cs index 70757a2ff4b..4d564ab4d21 100644 --- a/src/System.Management.Automation/engine/remoting/server/OutOfProcServerMediator.cs +++ b/src/System.Management.Automation/engine/remoting/server/OutOfProcServerMediator.cs @@ -386,7 +386,9 @@ protected void Start(string initialCommand, PSRemotingCryptoHelperServer cryptoH catch (Exception e) { PSEtwLog.LogOperationalError( - PSEventId.TransportError, PSOpcode.Open, PSTask.None, + PSEventId.TransportError, + PSOpcode.Open, + PSTask.None, PSKeyword.UseAlwaysOperational, Guid.Empty.ToString(), Guid.Empty.ToString(), @@ -395,7 +397,9 @@ protected void Start(string initialCommand, PSRemotingCryptoHelperServer cryptoH e.StackTrace); PSEtwLog.LogAnalyticError( - PSEventId.TransportError_Analytic, PSOpcode.Open, PSTask.None, + PSEventId.TransportError_Analytic, + PSOpcode.Open, + PSTask.None, PSKeyword.Transport | PSKeyword.UseAlwaysAnalytic, Guid.Empty.ToString(), Guid.Empty.ToString(), @@ -483,7 +487,7 @@ private OutOfProcessMediator() : base(true) #region Static Methods /// - /// Starts the out-of-process powershell server instance. + /// Starts the out-of-process powershell server instance. /// /// Specifies the initialization script. /// Specifies the initial working directory. The working directory is set before the initial command. diff --git a/src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs b/src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs index b23bf8160f7..de9a903f06f 100644 --- a/src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs +++ b/src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs @@ -44,6 +44,9 @@ internal class ServerRunspacePoolDriver : IRSPDriverInvoke // local runspace pool at the server + // Optional initial location of the PowerShell session + private readonly string _initialLocation; + // Script to run after a RunspacePool/Runspace is created in this session. private ConfigurationDataFromXML _configData; @@ -87,9 +90,6 @@ private Dictionary _associatedShells // Results in a configured remote runspace pushed onto driver host. private string _configurationName; - // Optional initial location of the PowerShell session - private readonly string _initialLocation; - /// /// Event that get raised when the RunspacePool is closed. /// @@ -100,18 +100,17 @@ private Dictionary _associatedShells #region Constructors /// - /// Creates the runspace pool driver. + /// Initializes a new instance of the runspace pool driver. /// /// Client runspace pool id to associate. - /// transport manager associated with this - /// runspace pool driver + /// Transport manager associated with this + /// runspace pool driver. /// Maximum runspaces to open. /// Minimum runspaces to open. /// Threading options for the runspaces in the pool. /// Apartment state for the runspaces in the pool. /// Host information about client side host. - /// - /// Contains: + /// Contains: /// 1. Script to run after a RunspacePool/Runspace is created in this session. /// For RunspacePool case, every newly created Runspace (in the pool) will run /// this script. diff --git a/src/System.Management.Automation/engine/remoting/server/serverremotesession.cs b/src/System.Management.Automation/engine/remoting/server/serverremotesession.cs index f33368a0a1b..dc740ec962a 100644 --- a/src/System.Management.Automation/engine/remoting/server/serverremotesession.cs +++ b/src/System.Management.Automation/engine/remoting/server/serverremotesession.cs @@ -192,7 +192,8 @@ internal ServerRemoteSession(PSSenderInfo senderInfo, ... */ - internal static ServerRemoteSession CreateServerRemoteSession(PSSenderInfo senderInfo, + internal static ServerRemoteSession CreateServerRemoteSession( + PSSenderInfo senderInfo, string configurationProviderId, string initializationParameters, AbstractServerSessionTransportManager transportManager, From 0cd53ffb6a0f6bad8a74d099ac25d18d4ff1d1b4 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 21 Sep 2019 12:32:31 +0500 Subject: [PATCH 3/5] Next step --- .../engine/hostifaces/PSTask.cs | 31 ++++++++++--------- .../namespaces/FileSystemContentStream.cs | 26 +++++++++++----- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index 59652393b1a..e0adf784577 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -40,7 +40,7 @@ public PSTask( ScriptBlock scriptBlock, Dictionary usingValuesMap, object dollarUnderbar, - PSTaskDataStreamWriter dataStreamWriter) + PSTaskDataStreamWriter dataStreamWriter) : base( scriptBlock, usingValuesMap, @@ -127,7 +127,7 @@ private void HandleInformationData() new PSStreamObject(PSStreamObjectType.Information, item)); } } - + #endregion #region Event handlers @@ -361,8 +361,8 @@ public PSInvocationState State #region Constructor - private PSTaskBase() - { + private PSTaskBase() + { _id = Interlocked.Increment(ref s_taskId); } @@ -422,7 +422,7 @@ public void Start() // Create and open Runspace for this task to run in var iss = InitialSessionState.CreateDefault2(); - iss.LanguageMode = (SystemPolicy.GetSystemLockdownPolicy() == SystemEnforcementMode.Enforce) + iss.LanguageMode = (SystemPolicy.GetSystemLockdownPolicy() == SystemEnforcementMode.Enforce) ? PSLanguageMode.ConstrainedLanguage : PSLanguageMode.FullLanguage; _runspace = RunspaceFactory.CreateRunspace(iss); _runspace.Name = string.Format(CultureInfo.InvariantCulture, "{0}:{1}", RunspaceName, s_taskId); @@ -475,7 +475,7 @@ internal sealed class PSTaskDataStreamWriter : IDisposable private readonly PSCmdlet _cmdlet; private readonly PSDataCollection _dataStream; private readonly int _cmdletThreadId; - + #endregion #region Properties @@ -711,6 +711,7 @@ public bool Add(PSTaskBase task) task.Start(); } + return true; case Stop: @@ -739,7 +740,7 @@ public void StopAll() // Accept no more input Close(); _stopAll.Set(); - + // Stop all running tasks lock (_syncObject) { @@ -764,7 +765,7 @@ public void Close() #region Private Methods private void HandleTaskStateChangedDelegate(object sender, PSInvocationStateChangedEventArgs args) => HandleTaskStateChanged(sender, args); - + private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEventArgs args) { var task = sender as PSTaskBase; @@ -805,10 +806,10 @@ private void CheckForComplete() try { PoolComplete.SafeInvoke( - this, + this, new EventArgs()); } - catch + catch { Dbg.Assert(false, "Exceptions should not be thrown on event thread"); } @@ -872,8 +873,8 @@ public override string Location /// public override bool HasMoreData { - get - { + get + { foreach (var childJob in ChildJobs) { if (childJob.HasMoreData) @@ -901,7 +902,7 @@ public override void StopJob() { _stopSignaled = true; SetJobState(JobState.Stopping); - + _taskPool.StopAll(); SetJobState(JobState.Stopped); } @@ -953,7 +954,7 @@ public void Start() // This thread will end once all jobs reach a finished state by either running // to completion, terminating with error, or stopped. System.Threading.ThreadPool.QueueUserWorkItem( - (_) => + (_) => { foreach (var childJob in ChildJobs) { @@ -991,7 +992,7 @@ private void HandleTaskPoolComplete(object sender, EventArgs args) SetJobState(finalState); } - catch (ObjectDisposedException) + catch (ObjectDisposedException) { } } diff --git a/src/System.Management.Automation/namespaces/FileSystemContentStream.cs b/src/System.Management.Automation/namespaces/FileSystemContentStream.cs index e9cfefd50a4..4d7dae05950 100644 --- a/src/System.Management.Automation/namespaces/FileSystemContentStream.cs +++ b/src/System.Management.Automation/namespaces/FileSystemContentStream.cs @@ -472,8 +472,7 @@ internal void SeekItemsBackward(int backCount) { chars[i] = buf[j]; } - } - ); + }); long currentBlock = 0; string lastDelimiterMatch = null; @@ -572,18 +571,25 @@ private bool ReadByLine(bool waitChanges, List blocks, bool readBackward { WaitForChanges(_path, _mode, _access, _share, _reader.CurrentEncoding); line = _reader.ReadLine(); - } while ((line == null) && (!_provider.Stopping)); + } + while ((line == null) && (!_provider.Stopping)); } } if (line != null) + { blocks.Add(line); + } int peekResult = readBackward ? _backReader.Peek() : _reader.Peek(); if (peekResult == -1) + { return false; + } else + { return true; + } } private bool ReadDelimited(bool waitChanges, List blocks, bool readBackward, string actualDelimiter) @@ -636,7 +642,7 @@ private bool ReadDelimited(bool waitChanges, List blocks, bool readBackw // We only wait for changes when read forwards, so here we don't need to check if 'readBackward' is // true or false, we only use 'reader'. The member 'reader' will be updated by WaitForChanges. WaitForChanges(_path, _mode, _access, _share, _reader.CurrentEncoding); - numRead += _reader.Read(readBuffer.Slice(0, (currentOffset - numRead))); + numRead += _reader.Read(readBuffer.Slice(0, currentOffset - numRead)); } } } @@ -679,7 +685,8 @@ private bool ReadDelimited(bool waitChanges, List blocks, bool readBackw } } } - } while (delimiterNotFound && (numRead != 0)); + } + while (delimiterNotFound && (numRead != 0)); // We've reached the end of file or end of line. if (_currentLineContent.Length > 0) @@ -692,13 +699,14 @@ private bool ReadDelimited(bool waitChanges, List blocks, bool readBackw blocks.Add( !readBackward && !delimiterNotFound ? _currentLineContent.ToString(0, _currentLineContent.Length - actualDelimiter.Length) - : _currentLineContent.ToString() - ); + : _currentLineContent.ToString()); } int peekResult = readBackward ? _backReader.Peek() : _reader.Peek(); if (peekResult != -1) + { return true; + } else { if (readBackward && _currentLineContent.Length > 0) @@ -726,7 +734,9 @@ private bool ReadByteEncoded(bool waitChanges, List blocks, bool readBac // Break when the end of the file is reached. if (n == 0) + { break; + } numBytesRead += n; numBytesToRead -= n; @@ -778,7 +788,9 @@ private bool ReadByteEncoded(bool waitChanges, List blocks, bool readBac return true; } else + { return false; + } } private void CreateStreams(string filePath, string streamName, FileMode fileMode, FileAccess fileAccess, FileShare fileShare, Encoding fileEncoding) From c8aa33ae5ab6320ce70e0f8e11061dd3129482d3 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 21 Sep 2019 12:48:55 +0500 Subject: [PATCH 4/5] Fixes in Parser and Ast --- .../engine/parser/Parser.cs | 38 +++++++++++-------- .../engine/parser/ast.cs | 13 ++++--- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/System.Management.Automation/engine/parser/Parser.cs b/src/System.Management.Automation/engine/parser/Parser.cs index 28bed16b15e..b65bfb415d9 100644 --- a/src/System.Management.Automation/engine/parser/Parser.cs +++ b/src/System.Management.Automation/engine/parser/Parser.cs @@ -6621,14 +6621,16 @@ private ExpressionAst BinaryExpressionRule(bool endNumberOnTernaryOpChars = fals if (expr == null) { // ErrorRecovery: create an error expression to fill out the ast and keep parsing. - IScriptExtent extent = After(token); + // Use token.Text, not token.Kind.Text() b/c the kind might not match the actual operator used // when a case insensitive operator is used. - ReportIncompleteInput(extent, + ReportIncompleteInput( + extent, nameof(ParserStrings.ExpectedValueExpression), ParserStrings.ExpectedValueExpression, token.Text); + expr = new ErrorExpressionAst(extent); } @@ -6652,7 +6654,9 @@ private ExpressionAst BinaryExpressionRule(bool endNumberOnTernaryOpChars = fals Token op = operatorStack.Pop(); operandStack.Push(new BinaryExpressionAst(ExtentOf(lhs, rhs), lhs, op.Kind, rhs, op.Extent)); if (operatorStack.Count == 0) + { break; + } precedence = operatorStack.Peek().Kind.GetBinaryPrecedence(); } @@ -6710,7 +6714,6 @@ private ExpressionAst ArrayLiteralRule(bool endNumberOnTernaryOpChars = false) // G array-literal-expression: // G unary-expression // G unary-expression ',' new-lines:opt array-literal-expression - ExpressionAst lastExpr = UnaryExpressionRule(endNumberOnTernaryOpChars); if (lastExpr == null) { @@ -6737,7 +6740,6 @@ private ExpressionAst ArrayLiteralRule(bool endNumberOnTernaryOpChars = false) if (lastExpr == null) { // ErrorRecovery: create an error expression for the ast and break. - ReportIncompleteInput(After(commaToken), nameof(ParserStrings.MissingExpressionAfterToken), ParserStrings.MissingExpressionAfterToken, @@ -6851,13 +6853,15 @@ private ExpressionAst UnaryExpressionRule(bool endNumberOnTernaryOpChars = false { // ErrorRecovery: don't bother constructing a unary expression, but we know we must have // some sort of expression, so return an error expression. - + // // Use token.Text, not token.Kind.Text() b/c the kind might not match the actual operator used // when a case insensitive operator is used. - ReportIncompleteInput(After(token), + ReportIncompleteInput( + After(token), nameof(ParserStrings.MissingExpressionAfterOperator), ParserStrings.MissingExpressionAfterOperator, token.Text); + return new ErrorExpressionAst(token.Extent); } } @@ -6881,11 +6885,12 @@ private ExpressionAst UnaryExpressionRule(bool endNumberOnTernaryOpChars = false { // ErrorRecovery: We have a list of attributes, and we know it's not before a param statement, // so we know we must have some sort of expression. Return an error expression then. - - ReportIncompleteInput(lastAttribute.Extent, + ReportIncompleteInput( + lastAttribute.Extent, nameof(ParserStrings.UnexpectedAttribute), ParserStrings.UnexpectedAttribute, lastAttribute.TypeName.FullName); + return new ErrorExpressionAst(ExtentOf(token, lastAttribute)); } @@ -6893,16 +6898,18 @@ private ExpressionAst UnaryExpressionRule(bool endNumberOnTernaryOpChars = false } else { - Diagnostics.Assert(_ungotToken == null || ErrorList.Count > 0, - "Unexpected lookahead from AttributeListRule."); + Diagnostics.Assert( + _ungotToken == null || ErrorList.Count > 0, + "Unexpected lookahead from AttributeListRule."); + // If we've looked ahead, don't go looking for a member access token, we've already issued an error, // just assume we're not trying to access a member. var memberAccessToken = _ungotToken != null ? null : NextMemberAccessToken(false); if (memberAccessToken != null) { - expr = CheckPostPrimaryExpressionOperators(memberAccessToken, - new TypeExpressionAst(lastAttribute.Extent, - lastAttribute.TypeName)); + expr = CheckPostPrimaryExpressionOperators( + memberAccessToken, + new TypeExpressionAst(lastAttribute.Extent, lastAttribute.TypeName)); } else { @@ -6913,8 +6920,9 @@ private ExpressionAst UnaryExpressionRule(bool endNumberOnTernaryOpChars = false child = UnaryExpressionRule(endNumberOnTernaryOpChars: true); if (child != null) { - expr = new ConvertExpressionAst(ExtentOf(lastAttribute, child), - (TypeConstraintAst)lastAttribute, child); + expr = new ConvertExpressionAst( + ExtentOf(lastAttribute, child), + (TypeConstraintAst)lastAttribute, child); } } } diff --git a/src/System.Management.Automation/engine/parser/ast.cs b/src/System.Management.Automation/engine/parser/ast.cs index 5b7db6a625c..5c10509d38f 100644 --- a/src/System.Management.Automation/engine/parser/ast.cs +++ b/src/System.Management.Automation/engine/parser/ast.cs @@ -7104,7 +7104,7 @@ internal virtual bool ShouldPreserveOutputInCaseOfException() public class TernaryExpressionAst : ExpressionAst { /// - /// Construct a binary expression. + /// Initializes a new instance of the a ternary expression. /// /// The extent of the expression. /// The condition operand. @@ -7123,23 +7123,26 @@ public TernaryExpressionAst(IScriptExtent extent, ExpressionAst condition, Expre } /// - /// The ast for the condition of the ternary expression. The property is never null. + /// Gets the ast for the condition of the ternary expression. The property is never null. /// public ExpressionAst Condition { get; } /// - /// The ast for the if-operand of the ternary expression. The property is never null. + /// Gets the ast for the if-operand of the ternary expression. The property is never null. /// public ExpressionAst IfTrue { get; } /// - /// The ast for the else-operand of the ternary expression. The property is never null. + /// Gets the ast for the else-operand of the ternary expression. The property is never null. /// public ExpressionAst IfFalse { get; } /// /// Copy the TernaryExpressionAst instance. /// + /// + /// Retirns a copy of the ast. + /// public override Ast Copy() { ExpressionAst newCondition = CopyElement(this.Condition); @@ -7199,7 +7202,7 @@ internal override AstVisitAction InternalVisit(AstVisitor visitor) public class BinaryExpressionAst : ExpressionAst { /// - /// Construct a binary expression. + /// Initializes a new instance of the binary expression. /// /// The extent of the expression. /// The left hand operand. From dbee27884cfb021e89e7a15af765ff0a87c104d4 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 21 Sep 2019 12:55:36 +0500 Subject: [PATCH 5/5] Fix Telemetry.cs --- src/System.Management.Automation/utils/Telemetry.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/utils/Telemetry.cs b/src/System.Management.Automation/utils/Telemetry.cs index c7d40c1377e..aceb8fd5e28 100644 --- a/src/System.Management.Automation/utils/Telemetry.cs +++ b/src/System.Management.Automation/utils/Telemetry.cs @@ -104,12 +104,14 @@ static ApplicationInsightsTelemetry() { s_telemetryClient = new TelemetryClient(); TelemetryConfiguration.Active.InstrumentationKey = _psCoreTelemetryKey; + // Set this to true to reduce latency during development TelemetryConfiguration.Active.TelemetryChannel.DeveloperMode = false; s_sessionId = Guid.NewGuid().ToString(); // use a hashset when looking for module names, it should be quicker than a string comparison - s_knownModules = new HashSet(StringComparer.OrdinalIgnoreCase) { + s_knownModules = new HashSet(StringComparer.OrdinalIgnoreCase) + { "Microsoft.PowerShell.Archive", "Microsoft.PowerShell.Host", "Microsoft.PowerShell.Management", @@ -122,6 +124,7 @@ static ApplicationInsightsTelemetry() "PSReadLine", "ThreadJob", }; + s_uniqueUserIdentifier = GetUniqueIdentifier().ToString(); } } @@ -193,7 +196,6 @@ internal static void SendTelemetryMetric(TelemetryType metricId, string data) // do nothing, telemetry can't be sent // don't send the panic telemetry as if we have failed above, it will likely fail here. } - } // Get the experimental feature name. If we can report it, we'll return the name of the feature, otherwise, we'll return "anonymous" @@ -233,7 +235,9 @@ internal static void SendPSCoreStartupTelemetry(string mode) { return; } + var properties = new Dictionary(); + // The variable POWERSHELL_DISTRIBUTION_CHANNEL is set in our docker images. // This allows us to track the actual docker OS as OSDescription provides only "linuxkit" // which has limited usefulness @@ -354,7 +358,7 @@ private static bool TryCreateUniqueIdentifierAndFile(string telemetryFilePath, o /// A guid which represents the unique identifier. private static Guid GetUniqueIdentifier() { - // Try to get the unique id. If this returns false, we'll + // Try to get the unique id. If this returns false, we'll // create/recreate the telemetry.uuid file to persist for next startup. Guid id = Guid.Empty; string uuidPath = Path.Join(Platform.CacheDirectory, "telemetry.uuid"); @@ -363,7 +367,7 @@ private static Guid GetUniqueIdentifier() return id; } - // Multiple processes may start simultaneously so we need a system wide + // Multiple processes may start simultaneously so we need a system wide // way to control access to the file in the case (although remote) when we have // simulataneous shell starts without the persisted file which attempt to create the file. using (var m = new Mutex(true, "CreateUniqueUserId"))