Expand variables on pipeline thread

So that expansions of embedded PowerShell code succeed.

Author: andyleejordan

src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs

@@ -241,11 +241,12 @@ public async Task<CommandBreakpointDetails[]> SetCommandBreakpointsAsync(
         /// that is identified by the given referenced ID.
         /// </summary>
         /// <param name="variableReferenceId"></param>
+        /// <param name="cancellationToken"></param>
         /// <returns>An array of VariableDetails instances which describe the requested variables.</returns>
-        public VariableDetailsBase[] GetVariables(int variableReferenceId)
+        public async Task<VariableDetailsBase[]> GetVariables(int variableReferenceId, CancellationToken cancellationToken)
         {
             VariableDetailsBase[] childVariables;
-            debugInfoHandle.Wait();
+            await debugInfoHandle.WaitAsync(cancellationToken).ConfigureAwait(false);
             try
             {
                 if ((variableReferenceId < 0) || (variableReferenceId >= variables.Count))
@@ -257,7 +258,12 @@ public VariableDetailsBase[] GetVariables(int variableReferenceId)
                 VariableDetailsBase parentVariable = variables[variableReferenceId];
                 if (parentVariable.IsExpandable)
                 {
-                    childVariables = parentVariable.GetChildren(_logger);
+                    // We execute this on the pipeline thread so the expansion of child variables works.
+                    childVariables = await _executionService.ExecuteDelegateAsync(
+                        $"Getting children of variable ${parentVariable.Name}",
+                        new ExecutionOptions { Priority = ExecutionPriority.Next },
+                        (_, _) => parentVariable.GetChildren(_logger), cancellationToken).ConfigureAwait(false);
+
                     foreach (VariableDetailsBase child in childVariables)
                     {
                         // Only add child if it hasn't already been added.
@@ -287,8 +293,9 @@ public VariableDetailsBase[] GetVariables(int variableReferenceId)
         /// walk the cached variable data for the specified stack frame.
         /// </summary>
         /// <param name="variableExpression">The variable expression string to evaluate.</param>
+        /// <param name="cancellationToken"></param>
         /// <returns>A VariableDetailsBase object containing the result.</returns>
-        public VariableDetailsBase GetVariableFromExpression(string variableExpression)
+        public async Task<VariableDetailsBase> GetVariableFromExpression(string variableExpression, CancellationToken cancellationToken)
         {
             // NOTE: From a watch we will get passed expressions that are not naked variables references.
             // Probably the right way to do this would be to examine the AST of the expr before calling
@@ -302,7 +309,7 @@ public VariableDetailsBase GetVariableFromExpression(string variableExpression)
             IEnumerable<VariableDetailsBase> variableList;
 
             // Ensure debug info isn't currently being built.
-            debugInfoHandle.Wait();
+            await debugInfoHandle.WaitAsync(cancellationToken).ConfigureAwait(false);
             try
             {
                 variableList = variables;
@@ -331,7 +338,7 @@ public VariableDetailsBase GetVariableFromExpression(string variableExpression)
                 if (resolvedVariable?.IsExpandable == true)
                 {
                     // Continue by searching in this variable's children.
-                    variableList = GetVariables(resolvedVariable.Id);
+                    variableList = await GetVariables(resolvedVariable.Id, cancellationToken).ConfigureAwait(false);
                 }
             }
 
@@ -491,18 +498,20 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
         /// <param name="writeResultAsOutput">
         /// If true, writes the expression result as host output rather than returning the results.
         /// In this case, the return value of this function will be null.</param>
+        /// <param name="cancellationToken"></param>
         /// <returns>A VariableDetails object containing the result.</returns>
         public async Task<VariableDetails> EvaluateExpressionAsync(
             string expressionString,
-            bool writeResultAsOutput)
+            bool writeResultAsOutput,
+            CancellationToken cancellationToken)
         {
             PSCommand command = new PSCommand().AddScript(expressionString);
             IReadOnlyList<PSObject> results;
             try
             {
                 results = await _executionService.ExecutePSCommandAsync<PSObject>(
                     command,
-                    CancellationToken.None,
+                    cancellationToken,
                     new PowerShellExecutionOptions { WriteOutputToHost = writeResultAsOutput, ThrowOnError = !writeResultAsOutput }).ConfigureAwait(false);
             }
             catch (Exception e)

src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs

@@ -229,7 +229,7 @@ private static string InsertDimensionSize(string value, int dimensionSize)
             return value + ": " + dimensionSize;
         }
 
-        private VariableDetails[] GetChildren(object obj, ILogger logger)
+        private static VariableDetails[] GetChildren(object obj, ILogger logger)
         {
             List<VariableDetails> childVariables = new();
 
@@ -238,86 +238,82 @@ private VariableDetails[] GetChildren(object obj, ILogger logger)
                 return childVariables.ToArray();
             }
 
-            try
-            {
-                PSObject psObject = obj as PSObject;
+            // NOTE: Variable expansion now takes place on the pipeline thread as an async delegate,
+            // so expansion of children that cause PowerShell script code to execute should
+            // generally work. However, we might need more error handling.
+            PSObject psObject = obj as PSObject;
 
-                if ((psObject != null) &&
-                    (psObject.TypeNames[0] == typeof(PSCustomObject).ToString()))
+            if ((psObject != null) &&
+                (psObject.TypeNames[0] == typeof(PSCustomObject).ToString()))
+            {
+                // PowerShell PSCustomObject's properties are completely defined by the ETS type system.
+                logger.LogDebug("PSObject was a PSCustomObject");
+                childVariables.AddRange(
+                    psObject
+                        .Properties
+                        .Select(p => new VariableDetails(p)));
+            }
+            else
+            {
+                // If a PSObject other than a PSCustomObject, unwrap it.
+                if (psObject != null)
                 {
-                    // PowerShell PSCustomObject's properties are completely defined by the ETS type system.
+                    // First add the PSObject's ETS properties
+                    logger.LogDebug("PSObject was something else, first getting ETS properties");
                     childVariables.AddRange(
                         psObject
                             .Properties
+                            // Here we check the object's MemberType against the `Properties`
+                            // bit-mask to determine if this is a property. Hence the selection
+                            // will only include properties.
+                            .Where(p => (PSMemberTypes.Properties & p.MemberType) is not 0)
                             .Select(p => new VariableDetails(p)));
+
+                    obj = psObject.BaseObject;
                 }
-                else
+
+                // We're in the realm of regular, unwrapped .NET objects
+                if (obj is IDictionary dictionary)
                 {
-                    // If a PSObject other than a PSCustomObject, unwrap it.
-                    if (psObject != null)
+                    logger.LogDebug("PSObject was an IDictionary");
+                    // Buckle up kids, this is a bit weird.  We could not use the LINQ
+                    // operator OfType<DictionaryEntry>.  Even though R# will squiggle the
+                    // "foreach" keyword below and offer to convert to a LINQ-expression - DON'T DO IT!
+                    // The reason is that LINQ extension methods work with objects of type
+                    // IEnumerable.  Objects of type Dictionary<,>, respond to iteration via
+                    // IEnumerable by returning KeyValuePair<,> objects.  Unfortunately non-generic
+                    // dictionaries like HashTable return DictionaryEntry objects.
+                    // It turns out that iteration via C#'s foreach loop, operates on the variable's
+                    // type which in this case is IDictionary.  IDictionary was designed to always
+                    // return DictionaryEntry objects upon iteration and the Dictionary<,> implementation
+                    // honors that when the object is reinterpreted as an IDictionary object.
+                    // FYI, a test case for this is to open $PSBoundParameters when debugging a
+                    // function that defines parameters and has been passed parameters.
+                    // If you open the $PSBoundParameters variable node in this scenario and see nothing,
+                    // this code is broken.
+                    foreach (DictionaryEntry entry in dictionary)
                     {
-                        // First add the PSObject's ETS properties
-                        childVariables.AddRange(
-                            psObject
-                                .Properties
-                                // Here we check the object's MemberType against the `Properties`
-                                // bit-mask to determine if this is a property. Hence the selection
-                                // will only include properties.
-                                .Where(p => (PSMemberTypes.Properties & p.MemberType) is not 0)
-                                .Select(p => new VariableDetails(p)));
-
-                        obj = psObject.BaseObject;
+                        childVariables.Add(
+                            new VariableDetails(
+                                "[" + entry.Key + "]",
+                                entry));
                     }
-
-                    // We're in the realm of regular, unwrapped .NET objects
-                    if (obj is IDictionary dictionary)
-                    {
-                        // Buckle up kids, this is a bit weird.  We could not use the LINQ
-                        // operator OfType<DictionaryEntry>.  Even though R# will squiggle the
-                        // "foreach" keyword below and offer to convert to a LINQ-expression - DON'T DO IT!
-                        // The reason is that LINQ extension methods work with objects of type
-                        // IEnumerable.  Objects of type Dictionary<,>, respond to iteration via
-                        // IEnumerable by returning KeyValuePair<,> objects.  Unfortunately non-generic
-                        // dictionaries like HashTable return DictionaryEntry objects.
-                        // It turns out that iteration via C#'s foreach loop, operates on the variable's
-                        // type which in this case is IDictionary.  IDictionary was designed to always
-                        // return DictionaryEntry objects upon iteration and the Dictionary<,> implementation
-                        // honors that when the object is reinterpreted as an IDictionary object.
-                        // FYI, a test case for this is to open $PSBoundParameters when debugging a
-                        // function that defines parameters and has been passed parameters.
-                        // If you open the $PSBoundParameters variable node in this scenario and see nothing,
-                        // this code is broken.
-                        foreach (DictionaryEntry entry in dictionary)
-                        {
-                            childVariables.Add(
-                                new VariableDetails(
-                                    "[" + entry.Key + "]",
-                                    entry));
-                        }
-                    }
-                    else if (obj is IEnumerable enumerable and not string)
+                }
+                else if (obj is IEnumerable enumerable and not string)
+                {
+                    logger.LogDebug("PSObject was an IEnumerable");
+                    int i = 0;
+                    foreach (object item in enumerable)
                     {
-                        int i = 0;
-                        foreach (object item in enumerable)
-                        {
-                            childVariables.Add(
-                                new VariableDetails(
-                                    "[" + i++ + "]",
-                                    item));
-                        }
+                        childVariables.Add(
+                            new VariableDetails(
+                                "[" + i++ + "]",
+                                item));
                     }
-
-                    AddDotNetProperties(obj, childVariables);
                 }
-            }
-            catch (GetValueInvocationException ex)
-            {
-                // This exception occurs when accessing the value of a
-                // variable causes a script to be executed.  Right now
-                // we aren't loading children on the pipeline thread so
-                // this causes an exception to be raised.  In this case,
-                // just return an empty list of children.
-                logger.LogWarning($"Failed to get properties of variable {Name}, value invocation was attempted: {ex.Message}");
+
+                logger.LogDebug("Adding .NET properties to PSObject");
+                AddDotNetProperties(obj, childVariables);
             }
 
             return childVariables.ToArray();

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebugEvaluateHandler.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation.
+// Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
 using System;
@@ -49,7 +49,7 @@ public async Task<EvaluateResponseBody> Handle(EvaluateRequestArguments request,
             {
                 await _executionService.ExecutePSCommandAsync(
                     new PSCommand().AddScript(request.Expression),
-                    CancellationToken.None,
+                    cancellationToken,
                     new PowerShellExecutionOptions { WriteOutputToHost = true, ThrowOnError = false, AddToHistory = true }).HandleErrorsAsync(_logger).ConfigureAwait(false);
             }
             else
@@ -61,12 +61,13 @@ await _executionService.ExecutePSCommandAsync(
                 if (_debugContext.IsStopped)
                 {
                     // First check to see if the watch expression refers to a naked variable reference.
-                    result = _debugService.GetVariableFromExpression(request.Expression);
+                    result = await _debugService.GetVariableFromExpression(request.Expression, cancellationToken).ConfigureAwait(false);
 
                     // If the expression is not a naked variable reference, then evaluate the expression.
                     result ??= await _debugService.EvaluateExpressionAsync(
                         request.Expression,
-                        isFromRepl).ConfigureAwait(false);
+                        isFromRepl,
+                        cancellationToken).ConfigureAwait(false);
                 }
 
                 if (result != null)

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/VariablesHandler.cs

@@ -18,11 +18,9 @@ internal class VariablesHandler : IVariablesHandler
 
         public VariablesHandler(DebugService debugService) => _debugService = debugService;
 
-        public Task<VariablesResponse> Handle(VariablesArguments request, CancellationToken cancellationToken)
+        public async Task<VariablesResponse> Handle(VariablesArguments request, CancellationToken cancellationToken)
         {
-            VariableDetailsBase[] variables =
-                _debugService.GetVariables(
-                    (int)request.VariablesReference);
+            VariableDetailsBase[] variables = await _debugService.GetVariables((int)request.VariablesReference, cancellationToken).ConfigureAwait(false);
 
             VariablesResponse variablesResponse = null;
 
@@ -41,7 +39,7 @@ public Task<VariablesResponse> Handle(VariablesArguments request, CancellationTo
                 // TODO: This shouldn't be so broad
             }
 
-            return Task.FromResult(variablesResponse);
+            return variablesResponse;
         }
     }
 }