Skip to content

Fix debugging with PSES pipeline thread changes #1574

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.

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
Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
9e753a7
Remove extraneous JS file
rjmholt May 10, 2021
0f5f5e0
Debugging dependency injection
rjmholt Apr 20, 2021
744fd88
Get UI debugger running
rjmholt Apr 21, 2021
0c21f19
Resolve commented-out code
rjmholt May 10, 2021
4cc3f77
Improve queue implementation
rjmholt May 25, 2021
cdd4e3e
Add async error handling code
rjmholt May 10, 2021
248008a
Rework execution queue to allow correct cancellation
rjmholt Jun 21, 2021
a001af4
TEMP: Need to fix cancellation from PSRL scope
rjmholt Jun 28, 2021
b5b3325
TEMP: Moving to monolithic host
rjmholt Jul 20, 2021
5428be9
WIP
rjmholt Aug 4, 2021
d3f8e01
Move all basic functionality into a new synchronous internal host
rjmholt Aug 13, 2021
502e4bb
Restore host to simple working order
rjmholt Aug 24, 2021
9ddfd9c
Add simple cancellation support
rjmholt Aug 30, 2021
942e7a6
Ensure startup operations are performed before first prompt
rjmholt Aug 30, 2021
ec9517f
Fix dependency issue
rjmholt Aug 30, 2021
d50ea57
Fix incorrect remote runspace labelling
rjmholt Aug 30, 2021
22d6088
Begin to make the debugger work
rjmholt Aug 30, 2021
6883f1e
Fix recursive PSRL cancellation token issue in debugger
rjmholt Sep 1, 2021
5fa2c4a
Enable proper debug UI interaction
rjmholt Sep 2, 2021
aaf27f7
Fix remote session connection
rjmholt Sep 3, 2021
d4ec378
Check the current runspace for debug API support
rjmholt Sep 8, 2021
b0af038
Fix debug disconnect handling
rjmholt Sep 9, 2021
a1c94be
Make remote debugger function correctly
rjmholt Sep 14, 2021
b5aab6e
Remove dead code
rjmholt Sep 15, 2021
b66cb0a
Remove more dead code
rjmholt Sep 15, 2021
e8bc341
Add comment about remote debugging
rjmholt Sep 15, 2021
822f0cc
Rename event handler
rjmholt Sep 15, 2021
a08855f
Add ConfigureAwait(false)
rjmholt Sep 15, 2021
797d38b
Make psEditor variable name a constant
rjmholt Sep 15, 2021
c8a691f
Remove debug cancellation token
rjmholt Sep 15, 2021
a6789aa
Remove duplicated check
rjmholt Sep 15, 2021
2d16735
Add comment
rjmholt Sep 15, 2021
b39e1fa
Add data structure for interlocked latch pattern
rjmholt Sep 15, 2021
298cbf1
Move cancellation token argument to end of methods
rjmholt Sep 15, 2021
e9521ff
Make BlockingConcurrentDeque disposable
rjmholt Sep 15, 2021
c9b782f
Rename PSES InternalHost to PsesInternalHost
rjmholt Oct 5, 2021
c39e298
Change WriteErrorsToHost to ThrowOnError
rjmholt Oct 5, 2021
fc4f0f4
Make default PowerShellExecutionOptions static
rjmholt Oct 7, 2021
fdb3ea3
Remove PowerShellContextService
rjmholt Oct 7, 2021
bbf838c
Ensure ConfigureAwait(false) is used everywhere
rjmholt Oct 7, 2021
fbdd818
Fix debugger evaluation not being written out
rjmholt Oct 7, 2021
bd07885
Add comment about nameless variables
rjmholt Oct 7, 2021
3207481
Use nicer pattern matching for condition
rjmholt Oct 7, 2021
98272ff
Add TODO about debugger disconnection handling
rjmholt Oct 7, 2021
2230ef9
Convert to `is null` for consistency
rjmholt Oct 7, 2021
bd19fae
Remove unused dependencies
rjmholt Oct 7, 2021
1fd44f6
Use AllThreadsContinued consistently
rjmholt Oct 7, 2021
22868f6
Add comment about debug context lifetime
rjmholt Oct 7, 2021
09b962d
Add context to comment
rjmholt Oct 7, 2021
e19878a
Fix breakpoint API test
rjmholt Oct 8, 2021
c2797d1
Fix ThrowOnError configuration check
rjmholt Oct 8, 2021
0f62b1d
Ensure the DSC module import fails with an error
rjmholt Oct 8, 2021
1a62125
Remove unneeded async/await
rjmholt Oct 8, 2021
0480ab6
Add comment explaining remote debug setting
rjmholt Oct 8, 2021
1d1cab8
Remove unused enum
rjmholt Oct 8, 2021
029539a
Add comment to execution options class
rjmholt Oct 8, 2021
03f18aa
Add comment to synchronous task about synchronous results
rjmholt Oct 8, 2021
a07e7da
Ensure F8 works
rjmholt Oct 8, 2021
25d38a1
Add comment about not exiting the top level
rjmholt Oct 8, 2021
268eb5f
Generalize exit handling
rjmholt Oct 8, 2021
15b16f0
Add comment about remote prompt
rjmholt Oct 9, 2021
1e98e9c
Simplify nested PowerShell creation
rjmholt Oct 9, 2021
d21efeb
Use a more explicit return
rjmholt Oct 9, 2021
543dabe
Fix Ctrl-C detection
rjmholt Oct 9, 2021
963fba1
Rename debug event handlers
rjmholt Oct 11, 2021
f5b644e
Remove unneeded PSRL static ctor call - PSRL is now loaded on the pip…
rjmholt Oct 11, 2021
905686c
Move execution service to an interface
rjmholt Oct 11, 2021
0a69937
Enhance remoting comment
rjmholt Oct 11, 2021
da786c8
Turn on psedit registration in remote sessions
rjmholt Oct 11, 2021
fb08b67
Ensure failed remote file saves are logged
rjmholt Oct 11, 2021
1f52dde
Add comment about IsExternalInit
rjmholt Oct 11, 2021
27a1735
Reinstate runspace cleanup logic
rjmholt Oct 11, 2021
87022dc
Fix remote psedit registration
rjmholt Oct 11, 2021
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
13 changes: 0 additions & 13 deletions ex.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private async Task CreateEditorServicesAndRunUntilShutdown()
_logger.Log(PsesLogLevel.Diagnostic, "Creating/running editor services");

bool creatingLanguageServer = _config.LanguageServiceTransport != null;
bool creatingDebugServer = false;// _config.DebugServiceTransport != null;
bool creatingDebugServer = _config.DebugServiceTransport != null;
bool isTempDebugSession = creatingDebugServer && !creatingLanguageServer;

// Set up information required to instantiate servers
Expand Down
12 changes: 9 additions & 3 deletions src/PowerShellEditorServices/Extensions/Api/EditorUIService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ public async Task<string> PromptInputAsync(string message)
new ShowInputPromptRequest
{
Name = message,
}).Returning<ShowInputPromptResponse>(CancellationToken.None);
})
.Returning<ShowInputPromptResponse>(CancellationToken.None)
.ConfigureAwait(false);

if (response.PromptCancelled)
{
Expand All @@ -142,7 +144,9 @@ public async Task<IReadOnlyList<string>> PromptMultipleSelectionAsync(string mes
Message = message,
Choices = choiceDetails,
DefaultChoices = defaultChoiceIndexes?.ToArray(),
}).Returning<ShowChoicePromptResponse>(CancellationToken.None);
})
.Returning<ShowChoicePromptResponse>(CancellationToken.None)
.ConfigureAwait(false);

if (response.PromptCancelled)
{
Expand All @@ -168,7 +172,9 @@ public async Task<string> PromptSelectionAsync(string message, IReadOnlyList<Pro
Message = message,
Choices = choiceDetails,
DefaultChoices = defaultChoiceIndex > -1 ? new[] { defaultChoiceIndex } : null,
}).Returning<ShowChoicePromptResponse>(CancellationToken.None);
})
.Returning<ShowChoicePromptResponse>(CancellationToken.None)
.ConfigureAwait(false);

if (response.PromptCancelled)
{
Expand Down
2 changes: 1 addition & 1 deletion src/PowerShellEditorServices/Extensions/EditorObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public EditorContext GetEditorContext()
internal void SetAsStaticInstance()
{
EditorObject.Instance = this;
s_editorObjectReady.SetResult(true);
s_editorObjectReady.TrySetResult(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to know what race condition this indicated.

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
<ItemGroup>
<Compile Remove="Extensions\Api\DocumentSymbolService.cs" />
<Compile Remove="Services\Extension\Templating\**" />
<Compile Remove="Services\PowerShellContext\**\*.cs" />
<EmbeddedResource Remove="Services\Extension\Templating\**" />
<None Remove="Services\Extension\Templating\**" />
</ItemGroup>
Expand Down
29 changes: 18 additions & 11 deletions src/PowerShellEditorServices/Server/PsesDebugServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@
using Microsoft.PowerShell.EditorServices.Handlers;
using Microsoft.PowerShell.EditorServices.Services;
using Microsoft.PowerShell.EditorServices.Services.PowerShell;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Console;
using Microsoft.PowerShell.EditorServices.Utility;
using OmniSharp.Extensions.DebugAdapter.Protocol;
using OmniSharp.Extensions.DebugAdapter.Protocol.Serialization;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host;
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace;
using OmniSharp.Extensions.DebugAdapter.Server;
using OmniSharp.Extensions.JsonRpc;
using OmniSharp.Extensions.LanguageServer.Server;

namespace Microsoft.PowerShell.EditorServices.Server
Expand Down Expand Up @@ -45,7 +43,7 @@ internal class PsesDebugServer : IDisposable

private DebugAdapterServer _debugAdapterServer;

private PowerShellExecutionService _executionService;
private PowerShellDebugContext _debugContext;

protected readonly ILoggerFactory _loggerFactory;

Expand Down Expand Up @@ -78,7 +76,8 @@ public async Task StartAsync()
{
// We need to let the PowerShell Context Service know that we are in a debug session
// so that it doesn't send the powerShell/startDebugger message.
_executionService = ServiceProvider.GetService<PowerShellExecutionService>();
_debugContext = ServiceProvider.GetService<PsesInternalHost>().DebugContext;
_debugContext.IsDebugServerActive = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI in master we had more logic added around this, so be careful when merging.


/*
// Needed to make sure PSReadLine's static properties are initialized in the pipeline thread.
Expand All @@ -99,10 +98,11 @@ public async Task StartAsync()
options
.WithInput(_inputStream)
.WithOutput(_outputStream)
.WithServices(serviceCollection => serviceCollection
.AddLogging()
.AddOptions()
.AddPsesDebugServices(ServiceProvider, this, _useTempSession))
.WithServices(serviceCollection =>
serviceCollection
.AddLogging()
.AddOptions()
.AddPsesDebugServices(ServiceProvider, this, _useTempSession))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a funny function...

// TODO: Consider replacing all WithHandler with AddSingleton
.WithHandler<LaunchAndAttachHandler>()
.WithHandler<DisconnectHandler>()
Expand All @@ -119,6 +119,9 @@ public async Task StartAsync()
// The OnInitialize delegate gets run when we first receive the _Initialize_ request:
// https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Initialize
.OnInitialize(async (server, request, cancellationToken) => {
// Ensure the debugger mode is set correctly - this is required for remote debugging to work
_debugContext.EnableDebugMode();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment for self: want to see what this method does.


var breakpointService = server.GetService<BreakpointService>();
// Clear any existing breakpoints before proceeding
await breakpointService.RemoveAllBreakpointsAsync().ConfigureAwait(false);
Expand All @@ -140,6 +143,10 @@ public async Task StartAsync()

public void Dispose()
{
// Note that the lifetime of the DebugContext is longer than the debug server;
// It represents the debugger on the PowerShell process we're in,
// while a new debug server is spun up for every debugging session
_debugContext.IsDebugServerActive = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm curious if we should make _debugContext disposable and have it handle its own cleanup.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's document that this is because the _debugContext's lifetime is greater than the object being disposed of here.

_debugAdapterServer.Dispose();
_inputStream.Dispose();
_outputStream.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ public static IServiceCollection AddPsesLanguageServices(
.AddSingleton<HostStartupInfo>(hostStartupInfo)
.AddSingleton<WorkspaceService>()
.AddSingleton<SymbolsService>()
.AddSingleton<EditorServicesConsolePSHost>()
.AddSingleton<PsesInternalHost>()
.AddSingleton<IRunspaceContext>(
(provider) => provider.GetService<EditorServicesConsolePSHost>())
.AddSingleton<PowerShellExecutionService>(
(provider) => provider.GetService<EditorServicesConsolePSHost>().ExecutionService)
(provider) => provider.GetService<PsesInternalHost>())
.AddSingleton<PowerShellExecutionService>()
.AddSingleton<ConfigurationService>()
.AddSingleton<IPowerShellDebugContext>(
(provider) => provider.GetService<EditorServicesConsolePSHost>().DebugContext)
(provider) => provider.GetService<PsesInternalHost>().DebugContext)
.AddSingleton<TemplateService>()
.AddSingleton<EditorOperationsService>()
.AddSingleton<RemoteFileManagerService>()
Expand Down Expand Up @@ -65,7 +64,13 @@ public static IServiceCollection AddPsesDebugServices(
PsesDebugServer psesDebugServer,
bool useTempSession)
{
return collection.AddSingleton(languageServiceProvider.GetService<PowerShellExecutionService>())
PsesInternalHost internalHost = languageServiceProvider.GetService<PsesInternalHost>();

return collection
.AddSingleton<PsesInternalHost>(internalHost)
.AddSingleton<IRunspaceContext>(internalHost)
.AddSingleton<IPowerShellDebugContext>(internalHost.DebugContext)
.AddSingleton(languageServiceProvider.GetService<PowerShellExecutionService>())
.AddSingleton(languageServiceProvider.GetService<WorkspaceService>())
.AddSingleton(languageServiceProvider.GetService<RemoteFileManagerService>())
.AddSingleton<PsesDebugServer>(psesDebugServer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ internal class BreakpointService
{
private readonly ILogger<BreakpointService> _logger;
private readonly PowerShellExecutionService _executionService;
private readonly EditorServicesConsolePSHost _editorServicesHost;
private readonly PsesInternalHost _editorServicesHost;
private readonly DebugStateService _debugStateService;

// TODO: This needs to be managed per nested session
Expand All @@ -33,7 +33,7 @@ internal class BreakpointService
public BreakpointService(
ILoggerFactory factory,
PowerShellExecutionService executionService,
EditorServicesConsolePSHost editorServicesHost,
PsesInternalHost editorServicesHost,
DebugStateService debugStateService)
{
_logger = factory.CreateLogger<BreakpointService>();
Expand All @@ -44,23 +44,23 @@ public BreakpointService(

public async Task<List<Breakpoint>> GetBreakpointsAsync()
{
if (BreakpointApiUtils.SupportsBreakpointApis)
if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
{
return BreakpointApiUtils.GetBreakpoints(
_editorServicesHost.Runspace.Debugger,
_debugStateService.RunspaceId);
}

// Legacy behavior
PSCommand psCommand = new PSCommand();
psCommand.AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint");
IEnumerable<Breakpoint> breakpoints = await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, new PowerShellExecutionOptions(), CancellationToken.None);
PSCommand psCommand = new PSCommand()
.AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint");
IEnumerable<Breakpoint> breakpoints = await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None).ConfigureAwait(false);
return breakpoints.ToList();
}

public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string escapedScriptPath, IEnumerable<BreakpointDetails> breakpoints)
{
if (BreakpointApiUtils.SupportsBreakpointApis)
if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
{
foreach (BreakpointDetails breakpointDetails in breakpoints)
{
Expand Down Expand Up @@ -140,7 +140,7 @@ public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string esc
if (psCommand != null)
{
IEnumerable<Breakpoint> setBreakpoints =
await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, new PowerShellExecutionOptions(), CancellationToken.None);
await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None).ConfigureAwait(false);
configuredBreakpoints.AddRange(
setBreakpoints.Select(BreakpointDetails.Create));
}
Expand All @@ -150,7 +150,7 @@ public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string esc

public async Task<IEnumerable<CommandBreakpointDetails>> SetCommandBreakpoints(IEnumerable<CommandBreakpointDetails> breakpoints)
{
if (BreakpointApiUtils.SupportsBreakpointApis)
if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
{
foreach (CommandBreakpointDetails commandBreakpointDetails in breakpoints)
{
Expand Down Expand Up @@ -217,7 +217,7 @@ public async Task<IEnumerable<CommandBreakpointDetails>> SetCommandBreakpoints(I
if (psCommand != null)
{
IEnumerable<Breakpoint> setBreakpoints =
await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, new PowerShellExecutionOptions(), CancellationToken.None);
await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None).ConfigureAwait(false);
configuredBreakpoints.AddRange(
setBreakpoints.Select(CommandBreakpointDetails.Create));
}
Expand All @@ -232,7 +232,7 @@ public async Task RemoveAllBreakpointsAsync(string scriptPath = null)
{
try
{
if (BreakpointApiUtils.SupportsBreakpointApis)
if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
{
foreach (Breakpoint breakpoint in BreakpointApiUtils.GetBreakpoints(
_editorServicesHost.Runspace.Debugger,
Expand Down Expand Up @@ -262,7 +262,7 @@ public async Task RemoveAllBreakpointsAsync(string scriptPath = null)

psCommand.AddCommand(@"Microsoft.PowerShell.Utility\Remove-PSBreakpoint");

await _executionService.ExecutePSCommandAsync<object>(psCommand, new PowerShellExecutionOptions(), CancellationToken.None).ConfigureAwait(false);
await _executionService.ExecutePSCommandAsync<object>(psCommand, CancellationToken.None).ConfigureAwait(false);
}
catch (Exception e)
{
Expand All @@ -272,7 +272,7 @@ public async Task RemoveAllBreakpointsAsync(string scriptPath = null)

public async Task RemoveBreakpointsAsync(IEnumerable<Breakpoint> breakpoints)
{
if (BreakpointApiUtils.SupportsBreakpointApis)
if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
{
foreach (Breakpoint breakpoint in breakpoints)
{
Expand Down Expand Up @@ -308,7 +308,7 @@ public async Task RemoveBreakpointsAsync(IEnumerable<Breakpoint> breakpoints)
psCommand.AddCommand(@"Microsoft.PowerShell.Utility\Remove-PSBreakpoint");
psCommand.AddParameter("Id", breakpoints.Select(b => b.Id).ToArray());

await _executionService.ExecutePSCommandAsync<object>(psCommand, new PowerShellExecutionOptions(), CancellationToken.None);
await _executionService.ExecutePSCommandAsync<object>(psCommand, CancellationToken.None).ConfigureAwait(false);
}
}

Expand Down
Loading