Skip to content

Fix several bugs in the debugger #1975

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
merged 5 commits into from
Dec 20, 2022
Merged

Fix several bugs in the debugger #1975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
243f2ea
Small suggested cleanups while browsing
andyleejordan Dec 17, 2022
e438e76
Expand variables on pipeline thread
andyleejordan Dec 17, 2022
89fb98f
Guard expansion of `PSPropertyInfo.Value`
andyleejordan Dec 19, 2022
874e1b8
Add test for custom `ToString` implementations
JustinGrote Dec 19, 2022
f6b1ffa
Run `debugService.Continue()` in the thread pool
andyleejordan Dec 20, 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
5 changes: 3 additions & 2 deletions src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#if DEBUG
using System.Diagnostics;
using System.Threading;

using Debugger = System.Diagnostics.Debugger;
#endif

Expand Down Expand Up @@ -197,9 +196,11 @@ protected override void BeginProcessing()
#if DEBUG
if (WaitForDebugger)
{
// NOTE: Ignore the suggestion to use Environment.ProcessId as it doesn't work for
// .NET 4.6.2 (for Windows PowerShell), and this won't be caught in CI.
Console.WriteLine($"Waiting for debugger to attach, PID: {Process.GetCurrentProcess().Id}");
while (!Debugger.IsAttached)
{
Console.WriteLine($"PID: {Process.GetCurrentProcess().Id}");
Thread.Sleep(1000);
}
}
Expand Down
25 changes: 17 additions & 8 deletions src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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)
Expand Down
187 changes: 94 additions & 93 deletions src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetails.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System;
Expand Down Expand Up @@ -68,7 +68,7 @@ public VariableDetails(PSObject psVariableObject)
/// The PSPropertyInfo instance from which variable details will be obtained.
/// </param>
public VariableDetails(PSPropertyInfo psProperty)
: this(psProperty.Name, psProperty.Value)
: this(psProperty.Name, SafeGetValue(psProperty))
{
}

Expand Down Expand Up @@ -98,16 +98,11 @@ public VariableDetails(string name, object value)
/// If this variable instance is expandable, this method returns the
/// details of its children. Otherwise it returns an empty array.
/// </summary>
/// <returns></returns>
public override VariableDetailsBase[] GetChildren(ILogger logger)
{
if (IsExpandable)
{
if (cachedChildren == null)
{
cachedChildren = GetChildren(ValueObject, logger);
}

cachedChildren ??= GetChildren(ValueObject, logger);
return cachedChildren;
}

Expand All @@ -118,6 +113,20 @@ public override VariableDetailsBase[] GetChildren(ILogger logger)

#region Private Methods

private static object SafeGetValue(PSPropertyInfo psProperty)
{
try
{
return psProperty.Value;
}
catch (GetValueInvocationException ex)
{
// Sometimes we can't get the value, like ExitCode, for reasons beyond our control,
// so just return the message from the exception that arises.
return new UnableToRetrievePropertyMessage { Name = psProperty.Name, Message = ex.Message };
}
}

private static bool GetIsExpandable(object valueObject)
{
if (valueObject == null)
Expand All @@ -131,9 +140,7 @@ private static bool GetIsExpandable(object valueObject)
valueObject = psobject.BaseObject;
}

Type valueType =
valueObject?.GetType();

Type valueType = valueObject?.GetType();
TypeInfo valueTypeInfo = valueType.GetTypeInfo();

return
Expand Down Expand Up @@ -236,7 +243,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();

Expand All @@ -245,86 +252,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
{
// If a PSObject other than a PSCustomObject, unwrap it.
if (psObject != null)
{
// 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;
}

// We're in the realm of regular, unwrapped .NET objects
if (obj is IDictionary dictionary)
// We're in the realm of regular, unwrapped .NET objects
if (obj is IDictionary dictionary)
{
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)
{
// 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));
}
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();
Expand All @@ -342,9 +345,8 @@ protected static void AddDotNetProperties(object obj, List<VariableDetails> chil
return;
}

PropertyInfo[] properties = objectType.GetProperties(BindingFlags.Public | BindingFlags.Instance);

foreach (PropertyInfo property in properties)
// Search all the public instance properties and add those missing.
foreach (PropertyInfo property in objectType.GetProperties(BindingFlags.Public | BindingFlags.Instance))
{
// Don't display indexer properties, it causes an exception anyway.
if (property.GetIndexParameters().Length > 0)
Expand All @@ -354,10 +356,11 @@ protected static void AddDotNetProperties(object obj, List<VariableDetails> chil

try
{
childVariables.Add(
new VariableDetails(
property.Name,
property.GetValue(obj)));
// Only add unique properties because we may have already added some.
if (!childVariables.Exists(p => p.Name == property.Name))
{
childVariables.Add(new VariableDetails(property.Name, property.GetValue(obj)));
}
}
catch (Exception ex)
{
Expand All @@ -371,21 +374,19 @@ protected static void AddDotNetProperties(object obj, List<VariableDetails> chil
childVariables.Add(
new VariableDetails(
property.Name,
new UnableToRetrievePropertyMessage(
"Error retrieving property - " + ex.GetType().Name)));
new UnableToRetrievePropertyMessage { Name = property.Name, Message = ex.Message }));
}
}
}

#endregion

private struct UnableToRetrievePropertyMessage
private record UnableToRetrievePropertyMessage
{
public UnableToRetrievePropertyMessage(string message) => Message = message;

public string Message { get; }
public string Name { get; init; }
public string Message { get; init; }

public override string ToString() => "<" + Message + ">";
public override string ToString() => $"Error retrieving property '${Name}': ${Message}";
}
}

Expand Down
1 change: 0 additions & 1 deletion src/PowerShellEditorServices/Services/DebugAdapter/Debugging/VariableDetailsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ internal abstract class VariableDetailsBase
/// If this variable instance is expandable, this method returns the
/// details of its children. Otherwise it returns an empty array.
/// </summary>
/// <returns></returns>
public abstract VariableDetailsBase[] GetChildren(ILogger logger);
}
}
Loading