From 07dc51b78a6bd7c6add1650ea7eff27397f4b3de Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Wed, 6 Aug 2025 10:27:21 +1000 Subject: [PATCH] Delay launch/attach until after config done Updates the sequence of launching and attaching scripts until after configuration done is received. This aligns the behaviour of starting a debug session with other debug adapters. A side effect of this change is that it is now possible to set breakpoints and do other actions in an attached runspace target during the initialisation. Also adds a new option `NotifyOnAttach` for an attach request which will create the `PSES.Attached` event before calling `Debug-Runspace` allowing attached scripts to wait for an attach event. --- .../DebugAdapter/BreakpointService.cs | 200 +++++++++++++++++- .../DebugAdapter/DebugEventHandlerService.cs | 11 - .../DebugAdapter/DebugStateService.cs | 79 ++++++- .../Handlers/ConfigurationDoneHandler.cs | 138 +----------- .../Handlers/LaunchAndAttachHandler.cs | 140 +++++++++++- .../DebugAdapterProtocolMessageTests.cs | 173 +++++++++++++-- .../Debugging/DebugServiceTests.cs | 18 +- 7 files changed, 562 insertions(+), 197 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs index 6d7e0c31a..578cf1f70 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs @@ -18,6 +18,154 @@ namespace Microsoft.PowerShell.EditorServices.Services { internal class BreakpointService { + private const string _getPSBreakpointLegacy = @" + [CmdletBinding()] + param ( + [Parameter()] + [string] + $Script, + + [Parameter()] + [int] + $RunspaceId = [Runspace]::DefaultRunspace.Id + ) + + $runspace = if ($PSBoundParameters.ContainsKey('RunspaceId')) { + Get-Runspace -Id $RunspaceId + $null = $PSBoundParameters.Remove('RunspaceId') + } + else { + [Runspace]::DefaultRunspace + } + + $debugger = $runspace.Debugger + $getBreakpointsMeth = $debugger.GetType().GetMethod( + 'GetBreakpoints', + [System.Reflection.BindingFlags]'NonPublic, Public, Instance', + $null, + [type[]]@(), + $null) + + $runspaceIdProp = [System.Management.Automation.PSNoteProperty]::new( + 'RunspaceId', + $runspaceId) + + @( + if (-not $getBreakpointsMeth) { + if ($RunspaceId -ne [Runspace]::DefaultRunspace.Id) { + throw 'Failed to find GetBreakpoints method on Debugger.' + } + + Microsoft.PowerShell.Utility\Get-PSBreakpoint @PSBoundParameters + } + else { + $getBreakpointsMeth.Invoke($debugger, @()) | Where-Object { + if ($Script) { + $_.Script -eq $Script + } + else { + $true + } + } + } + ) | ForEach-Object { + $_.PSObject.Properties.Add($runspaceIdProp) + $_ + } + "; + + private const string _removePSBreakpointLegacy = @" + [CmdletBinding(DefaultParameterSetName = 'Breakpoint')] + param ( + [Parameter(Mandatory, ValueFromPipeline, ParameterSetName = 'Breakpoint')] + [System.Management.Automation.Breakpoint[]] + $Breakpoint, + + [Parameter(Mandatory, ValueFromPipeline, ParameterSetName = 'Id')] + [int[]] + $Id, + + [Parameter(ParameterSetName = 'Id')] + [int] + $RunspaceId = [Runspace]::DefaultRunspace.Id + ) + + begin { + $removeBreakpointMeth = [Runspace]::DefaultRunspace.Debugger.GetType().GetMethod( + 'RemoveBreakpoint', + [System.Reflection.BindingFlags]'NonPublic, Public, Instance', + $null, + [type[]]@([System.Management.Automation.Breakpoint]), + $null) + $getBreakpointMeth = [Runspace]::DefaultRunspace.Debugger.GetType().GetMethod( + 'GetBreakpoint', + [System.Reflection.BindingFlags]'NonPublic, Public, Instance', + $null, + [type[]]@([int]), + $null) + + $breakpointCollection = [System.Collections.Generic.List[System.Management.Automation.Breakpoint]]::new() + } + + process { + if ($PSCmdlet.ParameterSetName -eq 'Id') { + $runspace = Get-Runspace -Id $RunspaceId + $runspaceProp = [System.Management.Automation.PSNoteProperty]::new( + 'Runspace', + $Runspace) + + $breakpoints = if ($getBreakpointMeth) { + foreach ($breakpointId in $Id) { + $getBreakpointMeth.Invoke($runspace.Debugger, @($breakpointId)) + } + } + elseif ($runspace -eq [Runspace]::DefaultRunspace) { + Microsoft.PowerShell.Utility\Get-PSBreakpoint -Id $Id + } + else { + throw 'Failed to find GetBreakpoint method on Debugger.' + } + + $breakpoints | ForEach-Object { + $_.PSObject.Properties.Add($runspaceProp) + $breakpointCollection.Add($_) + } + } + else { + foreach ($b in $Breakpoint) { + # RunspaceId may be set by _getPSBreakpointLegacy when + # targeting a breakpoint in a specific runspace. + $runspace = if ($b.PSObject.Properties.Match('RunspaceId')) { + Get-Runspace -Id $b.RunspaceId + } + else { + [Runspace]::DefaultRunspace + } + + $b.PSObject.Properties.Add( + [System.Management.Automation.PSNoteProperty]::new('Runspace', $runspace)) + $breakpointCollection.Add($b) + } + } + } + + end { + foreach ($b in $breakpointCollection) { + if ($removeBreakpointMeth) { + $removeBreakpointMeth.Invoke($b.Runspace.Debugger, @($b)) + } + elseif ($b.Runspace -eq [Runspace]::DefaultRunspace) { + # If we don't have the method, we can only remove breakpoints + # from the default runspace using Remove-PSBreakpoint. + $b | Microsoft.PowerShell.Utility\Remove-PSBreakpoint + } + else { + throw 'Failed to find RemoveBreakpoint method on Debugger.' + } + } + } + "; + /// /// Code used on WinPS 5.1 to set breakpoints without Script path validation. /// It uses reflection because the APIs were not public until 7.0 but just in @@ -45,7 +193,11 @@ internal class BreakpointService [Parameter(ParameterSetName = 'Command', Mandatory = $true)] [string] - $Command + $Command, + + [Parameter()] + [int] + $RunspaceId ) if ($Script) { @@ -65,6 +217,9 @@ internal class BreakpointService $null) if (-not $cmdCtor) { + if ($PSBoundParameters.ContainsKey('RunspaceId')) { + throw 'Failed to find constructor for CommandBreakpoint.' + } Microsoft.PowerShell.Utility\Set-PSBreakpoint @PSBoundParameters return } @@ -82,6 +237,9 @@ internal class BreakpointService $null) if (-not $lineCtor) { + if ($PSBoundParameters.ContainsKey('RunspaceId')) { + throw 'Failed to find constructor for LineBreakpoint.' + } Microsoft.PowerShell.Utility\Set-PSBreakpoint @PSBoundParameters return } @@ -89,8 +247,14 @@ internal class BreakpointService $b = $lineCtor.Invoke(@($Script, $Line, $Column, $Action)) } - [Runspace]::DefaultRunspace.Debugger.SetBreakpoints( - [System.Management.Automation.Breakpoint[]]@($b)) + $runspace = if ($PSBoundParameters.ContainsKey('RunspaceId')) { + Get-Runspace -Id $RunspaceId + } + else { + [Runspace]::DefaultRunspace + } + + $runspace.Debugger.SetBreakpoints([System.Management.Automation.Breakpoint[]]@($b)) $b "; @@ -128,10 +292,14 @@ public async Task> GetBreakpointsAsync() } // Legacy behavior - PSCommand psCommand = new PSCommand().AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint"); + PSCommand psCommand = new PSCommand().AddScript(_getPSBreakpointLegacy, useLocalScope: true); + if (_debugStateService.RunspaceId is not null) + { + psCommand.AddParameter("RunspaceId", _debugStateService.RunspaceId.Value); + } return await _executionService - .ExecutePSCommandAsync(psCommand, CancellationToken.None) - .ConfigureAwait(false); + .ExecutePSCommandAsync(psCommand, CancellationToken.None) + .ConfigureAwait(false); } public async Task> SetBreakpointsAsync(IReadOnlyList breakpoints) @@ -211,6 +379,11 @@ public async Task> SetBreakpointsAsync(IReadOnl { psCommand.AddParameter("Action", actionScriptBlock); } + + if (_debugStateService.RunspaceId is not null) + { + psCommand.AddParameter("RunspaceId", _debugStateService.RunspaceId.Value); + } } // If no PSCommand was created then there are no breakpoints to set. @@ -335,14 +508,17 @@ public async Task RemoveAllBreakpointsAsync(string scriptPath = null) } // Legacy behavior - PSCommand psCommand = new PSCommand().AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint"); - + PSCommand psCommand = new PSCommand().AddScript(_getPSBreakpointLegacy, useLocalScope: true); + if (_debugStateService.RunspaceId is not null) + { + psCommand.AddParameter("RunspaceId", _debugStateService.RunspaceId.Value); + } if (!string.IsNullOrEmpty(scriptPath)) { psCommand.AddParameter("Script", scriptPath); } - psCommand.AddCommand(@"Microsoft.PowerShell.Utility\Remove-PSBreakpoint"); + psCommand.AddScript(_removePSBreakpointLegacy, useLocalScope: true); await _executionService.ExecutePSCommandAsync(psCommand, CancellationToken.None).ConfigureAwait(false); } catch (Exception e) @@ -378,8 +554,12 @@ public async Task RemoveBreakpointsAsync(IEnumerable breakpoints) if (breakpointIds.Any()) { PSCommand psCommand = new PSCommand() - .AddCommand(@"Microsoft.PowerShell.Utility\Remove-PSBreakpoint") + .AddScript(_removePSBreakpointLegacy, useLocalScope: true) .AddParameter("Id", breakpoints.Select(b => b.Id).ToArray()); + if (_debugStateService.RunspaceId is not null) + { + psCommand.AddParameter("RunspaceId", _debugStateService.RunspaceId.Value); + } await _executionService.ExecutePSCommandAsync(psCommand, CancellationToken.None).ConfigureAwait(false); } } diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs index e3237a3b0..c887b02ac 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs @@ -94,17 +94,6 @@ private void OnRunspaceChanged(object sender, RunspaceChangedEventArgs e) { switch (e.ChangeAction) { - case RunspaceChangeAction.Enter: - if (_debugStateService.WaitingForAttach - && e.NewRunspace.RunspaceOrigin == RunspaceOrigin.DebuggedRunspace) - { - // Sends the InitializedEvent so that the debugger will continue - // sending configuration requests - _debugStateService.WaitingForAttach = false; - _debugStateService.ServerStarted.TrySetResult(true); - } - return; - case RunspaceChangeAction.Exit: if (_debugContext.IsStopped) { diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs index 9736b3e85..164ea44ca 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs @@ -4,27 +4,24 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.PowerShell.EditorServices.Utility; +using OmniSharp.Extensions.JsonRpc; namespace Microsoft.PowerShell.EditorServices.Services { internal class DebugStateService { private readonly SemaphoreSlim _setBreakpointInProgressHandle = AsyncUtils.CreateSimpleLockingSemaphore(); + private readonly SemaphoreSlim _inLaunchOrAttachHandle = AsyncUtils.CreateSimpleLockingSemaphore(); + private TaskCompletionSource _waitForConfigDone; internal bool NoDebug { get; set; } - internal string[] Arguments { get; set; } - internal bool IsRemoteAttach { get; set; } internal int? RunspaceId { get; set; } internal bool IsAttachSession { get; set; } - internal bool WaitingForAttach { get; set; } - - internal string ScriptToLaunch { get; set; } - internal bool ExecutionCompleted { get; set; } internal bool IsInteractiveDebugSession { get; set; } @@ -34,13 +31,79 @@ internal class DebugStateService internal bool IsUsingTempIntegratedConsole { get; set; } - internal string ExecuteMode { get; set; } - // This gets set at the end of the Launch/Attach handler which set debug state. internal TaskCompletionSource ServerStarted { get; set; } internal int ReleaseSetBreakpointHandle() => _setBreakpointInProgressHandle.Release(); internal Task WaitForSetBreakpointHandleAsync() => _setBreakpointInProgressHandle.WaitAsync(); + + /// + /// Sends the InitializedEvent and waits for the configuration done + /// event to be sent by the client. + /// + /// The action being performed, either "attach" or "launch". + /// A cancellation token to cancel the operation. + /// A launch or attach request is already in progress + internal async Task WaitForConfigurationDoneAsync( + string action, + CancellationToken cancellationToken) + { + Task waitForConfigDone; + + // First check we are not already running a launch or attach request. + await _inLaunchOrAttachHandle.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + if (_waitForConfigDone is not null) + { + // If we are already waiting for a configuration done, then we cannot start another + // launch or attach request. + throw new RpcErrorException(0, null, $"Cannot start a new {action} request when one is already in progress."); + } + + _waitForConfigDone = new TaskCompletionSource(); + waitForConfigDone = _waitForConfigDone.Task; + } + finally + { + _inLaunchOrAttachHandle.Release(); + } + + using CancellationTokenRegistration _ = cancellationToken.Register(_waitForConfigDone.SetCanceled); + + // Sends the InitializedEvent so that the debugger will continue + // sending configuration requests before the final configuration + // done. + ServerStarted.TrySetResult(true); + await waitForConfigDone.ConfigureAwait(false); + } + + /// + /// Sets the configuration done task to complete, indicating that the + /// client has sent all the initial configuration information and the + /// debugger is ready to start. + /// + /// A cancellation token to cancel the operation. + internal async Task SetConfigurationDoneAsync( + CancellationToken cancellationToken) + { + await _inLaunchOrAttachHandle.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + if (_waitForConfigDone is null) + { + // If we are not waiting for a configuration done, then we cannot set it. + throw new RpcErrorException(0, null, "Unexpected configuration done request when server is not expecting it."); + } + + _waitForConfigDone.TrySetResult(true); + _waitForConfigDone = null; + } + finally + { + _inLaunchOrAttachHandle.Release(); + } + } } } diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs index 146bbeae0..747b2d2c0 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs @@ -1,161 +1,35 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.Management.Automation; -using System.Management.Automation.Language; using System.Threading; using System.Threading.Tasks; -using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Services; -using Microsoft.PowerShell.EditorServices.Services.PowerShell; -using Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging; -using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution; -using Microsoft.PowerShell.EditorServices.Services.TextDocument; -using Microsoft.PowerShell.EditorServices.Utility; -using OmniSharp.Extensions.DebugAdapter.Protocol.Events; using OmniSharp.Extensions.DebugAdapter.Protocol.Requests; -using OmniSharp.Extensions.DebugAdapter.Protocol.Server; namespace Microsoft.PowerShell.EditorServices.Handlers { internal class ConfigurationDoneHandler : IConfigurationDoneHandler { - // TODO: We currently set `WriteInputToHost` as true, which writes our debugged commands' - // `GetInvocationText` and that reveals some obscure implementation details we should - // instead hide from the user with pretty strings (or perhaps not write out at all). - // - // This API is mostly used for F5 execution so it requires the foreground. - private static readonly PowerShellExecutionOptions s_debuggerExecutionOptions = new() - { - RequiresForeground = true, - WriteInputToHost = true, - WriteOutputToHost = true, - ThrowOnError = false, - AddToHistory = true, - }; - - private readonly ILogger _logger; - private readonly IDebugAdapterServerFacade _debugAdapterServer; private readonly DebugService _debugService; private readonly DebugStateService _debugStateService; - private readonly DebugEventHandlerService _debugEventHandlerService; - private readonly IInternalPowerShellExecutionService _executionService; - private readonly WorkspaceService _workspaceService; - private readonly IPowerShellDebugContext _debugContext; - // TODO: Decrease these arguments since they're a bunch of interfaces that can be simplified - // (i.e., `IRunspaceContext` should just be available on `IPowerShellExecutionService`). public ConfigurationDoneHandler( - ILoggerFactory loggerFactory, - IDebugAdapterServerFacade debugAdapterServer, DebugService debugService, - DebugStateService debugStateService, - DebugEventHandlerService debugEventHandlerService, - IInternalPowerShellExecutionService executionService, - WorkspaceService workspaceService, - IPowerShellDebugContext debugContext) + DebugStateService debugStateService) { - _logger = loggerFactory.CreateLogger(); - _debugAdapterServer = debugAdapterServer; _debugService = debugService; _debugStateService = debugStateService; - _debugEventHandlerService = debugEventHandlerService; - _executionService = executionService; - _workspaceService = workspaceService; - _debugContext = debugContext; } - public Task Handle(ConfigurationDoneArguments request, CancellationToken cancellationToken) + public async Task Handle(ConfigurationDoneArguments request, CancellationToken cancellationToken) { _debugService.IsClientAttached = true; - if (!string.IsNullOrEmpty(_debugStateService.ScriptToLaunch)) - { - // NOTE: This is an unawaited task because responding to "configuration done" means - // setting up the debugger, and in our case that means starting the script but not - // waiting for it to finish. - Task _ = LaunchScriptAsync(_debugStateService.ScriptToLaunch).HandleErrorsAsync(_logger); - } - - if (_debugStateService.IsInteractiveDebugSession && _debugService.IsDebuggerStopped) - { - if (_debugService.CurrentDebuggerStoppedEventArgs is not null) - { - // If this is an interactive session and there's a pending breakpoint, send that - // information along to the debugger client. - _debugEventHandlerService.TriggerDebuggerStopped(_debugService.CurrentDebuggerStoppedEventArgs); - } - else - { - // If this is an interactive session and there's a pending breakpoint that has - // not been propagated through the debug service, fire the debug service's - // OnDebuggerStop event. - _debugService.OnDebuggerStopAsync(null, _debugContext.LastStopEventArgs); - } - } - - return Task.FromResult(new ConfigurationDoneResponse()); - } - - // NOTE: We test this function in `DebugServiceTests` so it both needs to be internal, and - // use conditional-access on `_debugStateService` and `_debugAdapterServer` as its not set - // by tests. - internal async Task LaunchScriptAsync(string scriptToLaunch) - { - PSCommand command; - if (System.IO.File.Exists(scriptToLaunch)) - { - // For a saved file we just execute its path (after escaping it), with the configured operator - // (which can't be called that because it's a reserved keyword in C#). - string executeMode = _debugStateService?.ExecuteMode == "Call" ? "&" : "."; - command = PSCommandHelpers.BuildDotSourceCommandWithArguments( - PSCommandHelpers.EscapeScriptFilePath(scriptToLaunch), _debugStateService?.Arguments, executeMode); - } - else // It's a URI to an untitled script, or a raw script. - { - bool isScriptFile = _workspaceService.TryGetFile(scriptToLaunch, out ScriptFile untitledScript); - if (isScriptFile) - { - // Parse untitled files with their `Untitled:` URI as the filename which will - // cache the URI and contents within the PowerShell parser. By doing this, we - // light up the ability to debug untitled files with line breakpoints. - ScriptBlockAst ast = Parser.ParseInput( - untitledScript.Contents, - untitledScript.DocumentUri.ToString(), - out Token[] _, - out ParseError[] _); - - // In order to use utilize the parser's cache (and therefore hit line - // breakpoints) we need to use the AST's `ScriptBlock` object. Due to - // limitations in PowerShell's public API, this means we must use the - // `PSCommand.AddArgument(object)` method, hence this hack where we dot-source - // `$args[0]. Fortunately the dot-source operator maintains a stack of arguments - // on each invocation, so passing the user's arguments directly in the initial - // `AddScript` surprisingly works. - command = PSCommandHelpers - .BuildDotSourceCommandWithArguments("$args[0]", _debugStateService?.Arguments) - .AddArgument(ast.GetScriptBlock()); - } - else - { - // Without the new APIs we can only execute the untitled script's contents. - // Command breakpoints and `Wait-Debugger` will work. We must wrap the script - // with newlines so that any included comments don't break the command. - command = PSCommandHelpers.BuildDotSourceCommandWithArguments( - string.Concat( - "{" + System.Environment.NewLine, - isScriptFile ? untitledScript.Contents : scriptToLaunch, - System.Environment.NewLine + "}"), - _debugStateService?.Arguments); - } - } - - await _executionService.ExecutePSCommandAsync( - command, - CancellationToken.None, - s_debuggerExecutionOptions).ConfigureAwait(false); + // Tells the attach/launch request handler that the config is done + // and it can continue starting the script. + await _debugStateService.SetConfigurationDoneAsync(cancellationToken).ConfigureAwait(false); - _debugAdapterServer?.SendNotification(EventNames.Terminated); + return new ConfigurationDoneResponse(); } } } diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs index 200dcc316..300adc908 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Management.Automation; +using System.Management.Automation.Language; using System.Management.Automation.Remoting; using System.Threading; using System.Threading.Tasks; @@ -17,6 +18,7 @@ using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace; using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using Microsoft.PowerShell.EditorServices.Utility; using OmniSharp.Extensions.DebugAdapter.Protocol.Events; using OmniSharp.Extensions.DebugAdapter.Protocol.Requests; using OmniSharp.Extensions.DebugAdapter.Protocol.Server; @@ -99,10 +101,47 @@ internal record PsesAttachRequestArguments : AttachRequestArguments /// Gets or sets the path mappings for the remote debugging session. /// public PathMapping[] PathMappings { get; set; } = []; + + /// Gets or sets a boolean value that determines whether to write the + /// PSES.Attached event to the target runspace after attaching. + /// + public bool NotifyOnAttach { get; set; } } internal class LaunchAndAttachHandler : ILaunchHandler, IAttachHandler, IOnDebugAdapterServerStarted { + private const string _newAttachEventScript = @" + [CmdletBinding()] + param ( + [Parameter(Mandatory)] + [int] + $RunspaceId + ) + + $ErrorActionPreference = 'Stop' + + $runspace = Get-Runspace -Id $RunspaceId + $runspace.Events.GenerateEvent( + 'PSES.Attached', + 'PSES', + @(), + $null) + "; + + // TODO: We currently set `WriteInputToHost` as true, which writes our debugged commands' + // `GetInvocationText` and that reveals some obscure implementation details we should + // instead hide from the user with pretty strings (or perhaps not write out at all). + // + // This API is mostly used for F5 execution so it requires the foreground. + private static readonly PowerShellExecutionOptions s_debuggerExecutionOptions = new() + { + RequiresForeground = true, + WriteInputToHost = true, + WriteOutputToHost = true, + ThrowOnError = false, + AddToHistory = true, + }; + private static readonly int s_currentPID = System.Diagnostics.Process.GetCurrentProcess().Id; private static readonly Version s_minVersionForCustomPipeName = new(6, 2); private readonly ILogger _logger; @@ -110,6 +149,7 @@ internal class LaunchAndAttachHandler : ILaunchHandler(); + // DebugServiceTests will call this with a null DebugStateService. + if (_debugStateService is not null) + { + _debugStateService.ServerStarted = new TaskCompletionSource(); + } _remoteFileManagerService = remoteFileManagerService; } @@ -233,15 +279,21 @@ await _executionService.ExecutePSCommandAsync( _debugStateService.ScriptToLaunch = GetLaunchScript(request); _debugStateService.Arguments = request.Args; _debugStateService.IsUsingTempIntegratedConsole = request.CreateTemporaryIntegratedConsole; - _debugStateService.ExecuteMode = request.ExecuteMode; // If no script is being launched, mark this as an interactive // debugging session - _debugStateService.IsInteractiveDebugSession = string.IsNullOrEmpty(_debugStateService.ScriptToLaunch); + _debugStateService.IsInteractiveDebugSession = string.IsNullOrEmpty(requestScript); // Sends the InitializedEvent so that the debugger will continue // sending configuration requests - _debugStateService.ServerStarted.TrySetResult(true); + await _debugStateService.WaitForConfigurationDoneAsync("launch", cancellationToken).ConfigureAwait(false); + + if (!_debugStateService.IsInteractiveDebugSession) + { + // NOTE: This is an unawaited task because we are starting the script but not + // waiting for it to finish. + Task _ = LaunchScriptAsync(requestScript, request.Args, request.ExecuteMode).HandleErrorsAsync(_logger); + } return new LaunchResponse(); } @@ -362,16 +414,90 @@ void RunspaceChangedHandler(object s, RunspaceChangedEventArgs _) // Clear any existing breakpoints before proceeding await _breakpointService.RemoveAllBreakpointsAsync().ConfigureAwait(continueOnCapturedContext: false); - _debugStateService.WaitingForAttach = true; + // The debugger is now ready to receive breakpoint requests. We do + // this before running Debug-Runspace so the runspace is not busy + // and can set breakpoints before the final configuration done. + await _debugStateService.WaitForConfigurationDoneAsync("attach", cancellationToken).ConfigureAwait(false); + + if (request.NotifyOnAttach) + { + // This isn't perfect as there is still a race condition + // this and Debug-Runspace setting up the debugger below but it + // is the best we can do without changes to PowerShell. + await _executionService.ExecutePSCommandAsync( + new PSCommand().AddScript(_newAttachEventScript, useLocalScope: true) + .AddParameter("RunspaceId", _debugStateService.RunspaceId), + cancellationToken).ConfigureAwait(false); + } + Task nonAwaitedTask = _executionService .ExecutePSCommandAsync(debugRunspaceCmd, CancellationToken.None, PowerShellExecutionOptions.ImmediateInteractive) .ContinueWith(OnExecutionCompletedAsync, TaskScheduler.Default); - _debugStateService.ServerStarted.TrySetResult(true); - return new AttachResponse(); } + // NOTE: We test this function in `DebugServiceTests` so it both needs to be internal, and + // use conditional-access on `_debugStateService` and `_debugAdapterServer` as its not set + // by tests. + internal async Task LaunchScriptAsync(string scriptToLaunch, string[] arguments, string requestExecuteMode) + { + PSCommand command; + if (File.Exists(scriptToLaunch)) + { + // For a saved file we just execute its path (after escaping it), with the configured operator + // (which can't be called that because it's a reserved keyword in C#). + string executeMode = requestExecuteMode == "Call" ? "&" : "."; + command = PSCommandHelpers.BuildDotSourceCommandWithArguments( + PSCommandHelpers.EscapeScriptFilePath(scriptToLaunch), arguments, executeMode); + } + else // It's a URI to an untitled script, or a raw script. + { + bool isScriptFile = _workspaceService.TryGetFile(scriptToLaunch, out ScriptFile untitledScript); + if (isScriptFile) + { + // Parse untitled files with their `Untitled:` URI as the filename which will + // cache the URI and contents within the PowerShell parser. By doing this, we + // light up the ability to debug untitled files with line breakpoints. + ScriptBlockAst ast = Parser.ParseInput( + untitledScript.Contents, + untitledScript.DocumentUri.ToString(), + out Token[] _, + out ParseError[] _); + + // In order to use utilize the parser's cache (and therefore hit line + // breakpoints) we need to use the AST's `ScriptBlock` object. Due to + // limitations in PowerShell's public API, this means we must use the + // `PSCommand.AddArgument(object)` method, hence this hack where we dot-source + // `$args[0]. Fortunately the dot-source operator maintains a stack of arguments + // on each invocation, so passing the user's arguments directly in the initial + // `AddScript` surprisingly works. + command = PSCommandHelpers + .BuildDotSourceCommandWithArguments("$args[0]", arguments) + .AddArgument(ast.GetScriptBlock()); + } + else + { + // Without the new APIs we can only execute the untitled script's contents. + // Command breakpoints and `Wait-Debugger` will work. We must wrap the script + // with newlines so that any included comments don't break the command. + command = PSCommandHelpers.BuildDotSourceCommandWithArguments( + string.Concat( + "{" + Environment.NewLine, + isScriptFile ? untitledScript.Contents : scriptToLaunch, + Environment.NewLine + "}"), + arguments); + } + } + + await _executionService.ExecutePSCommandAsync( + command, + CancellationToken.None, + s_debuggerExecutionOptions).ConfigureAwait(false); + + _debugAdapterServer?.SendNotification(EventNames.Terminated); + } + private async Task AttachToComputer(string computerName, CancellationToken cancellationToken) { _debugStateService.IsRemoteAttach = true; diff --git a/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs b/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs index 1d8259ae5..ceed8a333 100644 --- a/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs +++ b/test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Linq; @@ -67,6 +68,12 @@ public class DebugAdapterProtocolMessageTests(ITestOutputHelper output) : IAsync /// private readonly TaskCompletionSource terminatedTcs = new(); + private readonly TaskCompletionSource serverInitializedTcs = new(); + /// + /// This task is useful for waiting until the server has sent the Initialized event. + /// + private Task serverInitialized => serverInitializedTcs.Task; + public async Task InitializeAsync() { // Cleanup testScriptLogPath if it exists due to an interrupted previous run @@ -130,6 +137,10 @@ send until a launch is sent. terminatedTcs.SetResult(e); return Task.CompletedTask; }) + .OnDebugAdapterInitialized((initEvent) => + { + serverInitializedTcs.SetResult(initEvent); + }) ; }); @@ -263,9 +274,11 @@ public void CanInitializeWithCorrectServerSettings() public async Task UsesDotSourceOperatorAndQuotesAsync() { string filePath = NewTestFile(GenerateLoggingScript("$($MyInvocation.Line)")); - await client.LaunchScript(filePath); + Task launchTask = client.LaunchScript(filePath); + await serverInitialized; ConfigurationDoneResponse configDoneResponse = await client.RequestConfigurationDone(new ConfigurationDoneArguments()); Assert.NotNull(configDoneResponse); + await launchTask; string actual = await ReadScriptLogLineAsync(); Assert.StartsWith(". '", actual); @@ -275,9 +288,11 @@ public async Task UsesDotSourceOperatorAndQuotesAsync() public async Task UsesCallOperatorWithSettingAsync() { string filePath = NewTestFile(GenerateLoggingScript("$($MyInvocation.Line)")); - await client.LaunchScript(filePath, executeMode: "Call"); + Task launchTask = client.LaunchScript(filePath, executeMode: "Call"); + await serverInitialized; ConfigurationDoneResponse configDoneResponse = await client.RequestConfigurationDone(new ConfigurationDoneArguments()); Assert.NotNull(configDoneResponse); + await launchTask; string actual = await ReadScriptLogLineAsync(); Assert.StartsWith("& '", actual); @@ -288,10 +303,12 @@ public async Task CanLaunchScriptWithNoBreakpointsAsync() { string filePath = NewTestFile(GenerateLoggingScript("works")); - await client.LaunchScript(filePath); + Task launchTask = client.LaunchScript(filePath); + await serverInitialized; ConfigurationDoneResponse configDoneResponse = await client.RequestConfigurationDone(new ConfigurationDoneArguments()); Assert.NotNull(configDoneResponse); + await launchTask; string actual = await ReadScriptLogLineAsync(); Assert.Equal("works", actual); @@ -309,7 +326,8 @@ public async Task CanSetBreakpointsAsync() "after breakpoint" )); - await client.LaunchScript(filePath); + Task launchTask = client.LaunchScript(filePath); + await serverInitialized; // {"command":"setBreakpoints","arguments":{"source":{"name":"dfsdfg.ps1","path":"/Users/tyleonha/Code/PowerShell/Misc/foo/dfsdfg.ps1"},"lines":[2],"breakpoints":[{"line":2}],"sourceModified":false},"type":"request","seq":3} SetBreakpointsResponse setBreakpointsResponse = await client.SetBreakpoints(new SetBreakpointsArguments @@ -327,6 +345,8 @@ public async Task CanSetBreakpointsAsync() ConfigurationDoneResponse configDoneResponse = await client.RequestConfigurationDone(new ConfigurationDoneArguments()); Assert.NotNull(configDoneResponse); + await launchTask; + // Wait until we hit the breakpoint StoppedEvent stoppedEvent = await nextStopped; Assert.Equal("breakpoint", stoppedEvent.Reason); @@ -370,8 +390,10 @@ await client.SetBreakpoints( ); // Signal to start the script + Task launchTask = client.LaunchScript(filePath); + await serverInitialized; await client.RequestConfigurationDone(new ConfigurationDoneArguments()); - await client.LaunchScript(filePath); + await launchTask; // Try to get the stacktrace, which should throw as we are not currently at a breakpoint. await Assert.ThrowsAsync(() => client.RequestStackTrace( @@ -390,7 +412,8 @@ public async Task SendsInitialLabelBreakpointForPerformanceReasons() )); // Request a launch. Note that per DAP spec, launch doesn't actually begin until ConfigDone finishes. - await client.LaunchScript(filePath); + Task launchTask = client.LaunchScript(filePath); + await serverInitialized; SetBreakpointsResponse setBreakpointsResponse = await client.SetBreakpoints(new SetBreakpointsArguments { @@ -406,6 +429,8 @@ public async Task SendsInitialLabelBreakpointForPerformanceReasons() _ = client.RequestConfigurationDone(new ConfigurationDoneArguments()); + await launchTask; + // Wait for the breakpoint to be hit StoppedEvent stoppedEvent = await nextStopped; Assert.Equal("breakpoint", stoppedEvent.Reason); @@ -452,7 +477,8 @@ public async Task CanStepPastSystemWindowsForms() "Write-Host $form" })); - await client.LaunchScript(filePath); + Task launchTask = client.LaunchScript(filePath); + await serverInitialized; SetFunctionBreakpointsResponse setBreakpointsResponse = await client.SetFunctionBreakpoints( new SetFunctionBreakpointsArguments @@ -466,6 +492,8 @@ public async Task CanStepPastSystemWindowsForms() ConfigurationDoneResponse configDoneResponse = await client.RequestConfigurationDone(new ConfigurationDoneArguments()); Assert.NotNull(configDoneResponse); + await launchTask; + await Task.Delay(5000); VariablesResponse variablesResponse = await client.RequestVariables( @@ -490,9 +518,11 @@ public async Task CanLaunchScriptWithCommentedLastLineAsync() // PsesLaunchRequestArguments.Script, which is then assigned to // DebugStateService.ScriptToLaunch in that handler, and finally used by the // ConfigurationDoneHandler in LaunchScriptAsync. - await client.LaunchScript(script); + Task launchTask = client.LaunchScript(script); + await serverInitialized; _ = await client.RequestConfigurationDone(new ConfigurationDoneArguments()); + await launchTask; // We can check that the script was invoked as expected, which is to dot-source a script // block with the contents surrounded by newlines. While we can't check that the last @@ -539,8 +569,10 @@ public async Task CanRunPesterTestFile() } }", isPester: true); - await client.LaunchScript($"Invoke-Pester -Script '{pesterTest}'"); + Task launchTask = client.LaunchScript($"Invoke-Pester -Script '{pesterTest}'"); + await serverInitialized; await client.RequestConfigurationDone(new ConfigurationDoneArguments()); + await launchTask; Assert.Equal("pester", await ReadScriptLogLineAsync()); } @@ -573,8 +605,10 @@ public async Task CanLaunchScriptWithNewChildAttachSession( terminatedTcs.TrySetCanceled(); }); - await client.LaunchScript(script); + Task launchTask = client.LaunchScript(script); + await serverInitialized; await client.RequestConfigurationDone(new ConfigurationDoneArguments()); + await launchTask; StartDebuggingAttachRequestArguments attachRequest = await startDebuggingAttachRequestTcs.Task; Assert.Equal("attach", attachRequest.Request); @@ -607,8 +641,10 @@ public async Task CanLaunchScriptWithNewChildAttachSessionAsJob() terminatedTcs.TrySetCanceled(); }); - await client.LaunchScript(script); + Task launchTask = client.LaunchScript(script); + await serverInitialized; await client.RequestConfigurationDone(new ConfigurationDoneArguments()); + await launchTask; StartDebuggingAttachRequestArguments attachRequest = await startDebuggingAttachRequestTcs.Task; Assert.Equal("attach", attachRequest.Request); @@ -621,6 +657,82 @@ public async Task CanLaunchScriptWithNewChildAttachSessionAsJob() await terminatedTcs.Task; } + [SkippableFact] + public async Task CanAttachScriptWithEventWait() + { + Skip.If(PsesStdioLanguageServerProcessHost.RunningInConstrainedLanguageMode, + "Breakpoints can't be set in Constrained Language Mode."); + + string[] logStatements = ["before breakpoint", "after breakpoint"]; + + await RunWithAttachableProcess(logStatements, async (filePath, processId, runspaceId) => + { + Task nextStoppedTask = nextStopped; + + Task attachTask = client.Attach( + new PsesAttachRequestArguments + { + ProcessId = processId, + RunspaceId = runspaceId, + NotifyOnAttach = true, + }); + + await serverInitialized; + + SetBreakpointsResponse setBreakpointsResponse = await client.SetBreakpoints(new SetBreakpointsArguments + { + Source = new Source { Name = Path.GetFileName(filePath), Path = filePath }, + Breakpoints = new SourceBreakpoint[] { new SourceBreakpoint { Line = 2 } }, + SourceModified = false, + }); + + Breakpoint breakpoint = setBreakpointsResponse.Breakpoints.First(); + Assert.True(breakpoint.Verified); + Assert.NotNull(breakpoint.Source); + Assert.Equal(filePath, breakpoint.Source.Path, ignoreCase: s_isWindows); + Assert.Equal(2, breakpoint.Line); + + ConfigurationDoneResponse configDoneResponse = await client.RequestConfigurationDone(new ConfigurationDoneArguments()); + Assert.NotNull(configDoneResponse); + + AttachResponse attachResponse = await attachTask; + Assert.NotNull(attachResponse); + + StoppedEvent stoppedEvent; + if (PsesStdioLanguageServerProcessHost.IsWindowsPowerShell) + { + // WinPS will break on first statement when Debug-Runspace + // is called. This does not happen in pwsh 7. + stoppedEvent = await nextStoppedTask; + Assert.Equal("step", stoppedEvent.Reason); + Assert.NotNull(stoppedEvent.ThreadId); + + nextStoppedTask = nextStopped; + + await client.RequestStackTrace(new StackTraceArguments { ThreadId = (int)stoppedEvent.ThreadId }); + await client.RequestContinue(new ContinueArguments { ThreadId = (int)stoppedEvent.ThreadId }); + } + + // Wait until we hit the breakpoint + stoppedEvent = await nextStopped; + Assert.Equal("breakpoint", stoppedEvent.Reason); + + // The code before the breakpoint should have already run + Assert.Equal("before breakpoint", await ReadScriptLogLineAsync()); + + // Assert that the stopped breakpoint is the one we set + StackTraceResponse stackTraceResponse = await client.RequestStackTrace(new StackTraceArguments { ThreadId = 1 }); + DapStackFrame? stoppedTopFrame = stackTraceResponse.StackFrames?.First(); + Assert.NotNull(stoppedTopFrame); + Assert.Equal(2, stoppedTopFrame.Line); + + _ = await client.RequestContinue(new ContinueArguments { ThreadId = 1 }); + + string afterBreakpointActual = await ReadScriptLogLineAsync(); + Assert.Equal("after breakpoint", afterBreakpointActual); + }, waitForNotifyEvent: true); + } + [SkippableFact] public async Task CanAttachScriptWithPathMappings() { @@ -638,7 +750,7 @@ await RunWithAttachableProcess(logStatements, async (filePath, processId, runspa Task nextStoppedTask = nextStopped; - AttachResponse attachResponse = await client.Attach( + Task attachTask = client.Attach( new PsesAttachRequestArguments { ProcessId = processId, @@ -650,8 +762,9 @@ await RunWithAttachableProcess(logStatements, async (filePath, processId, runspa RemoteRoot = Path.GetDirectoryName(filePath) + Path.DirectorySeparatorChar } ] - }) ?? throw new Exception("Attach response was null."); - Assert.NotNull(attachResponse); + }); + + await serverInitialized; SetBreakpointsResponse setBreakpointsResponse = await client.SetBreakpoints(new SetBreakpointsArguments { @@ -669,6 +782,9 @@ await RunWithAttachableProcess(logStatements, async (filePath, processId, runspa ConfigurationDoneResponse configDoneResponse = await client.RequestConfigurationDone(new ConfigurationDoneArguments()); Assert.NotNull(configDoneResponse); + AttachResponse attachResponse = await attachTask; + Assert.NotNull(attachResponse); + // Wait-Debugger stop StoppedEvent stoppedEvent = await nextStoppedTask; Assert.Equal("step", stoppedEvent.Reason); @@ -710,7 +826,7 @@ await RunWithAttachableProcess(logStatements, async (filePath, processId, runspa }); } - private async Task RunWithAttachableProcess(string[] logStatements, Func action) + private async Task RunWithAttachableProcess(string[] logStatements, Func action, bool waitForNotifyEvent = false) { /* There is no public API in pwsh to wait for an attach event. We @@ -727,7 +843,7 @@ once that is merged and we are running against that version but WinPS will always need this. */ string scriptEntrypoint = @" - param([string]$TestScript) + param([string]$TestScript, [string]$WaitScript) $debugRunspaceCmd = Get-Command Debug-Runspace -Module Microsoft.PowerShell.Utility $runspaceBase = [PSObject].Assembly.GetType( @@ -742,9 +858,12 @@ WinPS will always need this. $ps = [PowerShell]::Create() $runspace = $ps.Runspace - # Wait-Debugger is needed in WinPS to sync breakpoints before - # running the script. - $null = $ps.AddCommand('Wait-Debugger').AddStatement() + if ($WaitScript) { + $null = $ps.AddScript($WaitScript, $true).AddStatement() + } + else { + $null = $ps.AddCommand('Wait-Debugger').AddStatement() + } $null = $ps.AddCommand($TestScript) # Let the runner know what Runspace to attach to and that it @@ -778,7 +897,21 @@ WinPS will always need this. "; string filePath = NewTestFile(GenerateLoggingScript(logStatements)); - string encArgs = CreatePwshEncodedArgs(filePath); + + List args = [filePath]; + if (waitForNotifyEvent) + { + args.Add(@" + $e = Wait-Event -SourceIdentifier PSES.Attached -Timeout 10 + if ($e) { + $e | Remove-Event -Force + } + else { + throw 'Timed out waiting for PSES.Attached event.' + } + "); + } + string encArgs = CreatePwshEncodedArgs(args.ToArray()); string encCommand = Convert.ToBase64String(Encoding.Unicode.GetBytes(scriptEntrypoint)); ProcessStartInfo psi = new ProcessStartInfo diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index 3ba16008d..ab4f4ee49 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -552,10 +552,10 @@ await debugService.SetLineBreakpointsAsync( new[] { BreakpointDetails.Create(scriptPath, 1) }); } - ConfigurationDoneHandler configurationDoneHandler = new( - NullLoggerFactory.Instance, null, debugService, null, null, psesHost, workspace, null); + LaunchAndAttachHandler launchAndAttachHandler = new( + NullLoggerFactory.Instance, null, null, null, debugService, psesHost, psesHost, workspace, null, null); - Task _ = configurationDoneHandler.LaunchScriptAsync(scriptPath); + Task _ = launchAndAttachHandler.LaunchScriptAsync(scriptPath, [], "DotSource"); await AssertDebuggerStopped(scriptPath, 1); VariableDetailsBase[] variables = await GetVariables(VariableContainerDetails.CommandVariablesName); @@ -574,9 +574,9 @@ await debugService.SetLineBreakpointsAsync( [Fact] public async Task RecordsF5CommandInPowerShellHistory() { - ConfigurationDoneHandler configurationDoneHandler = new( - NullLoggerFactory.Instance, null, debugService, null, null, psesHost, workspace, null); - await configurationDoneHandler.LaunchScriptAsync(debugScriptFile.FilePath); + LaunchAndAttachHandler launchAndAttachHandler = new( + NullLoggerFactory.Instance, null, null, null, debugService, psesHost, psesHost, workspace, null, null); + await launchAndAttachHandler.LaunchScriptAsync(debugScriptFile.FilePath, [], "DotSource"); IReadOnlyList historyResult = await psesHost.ExecutePSCommandAsync( new PSCommand().AddScript("(Get-History).CommandLine"), @@ -614,9 +614,9 @@ public async Task RecordsF8CommandInHistory() [Fact] public async Task OddFilePathsLaunchCorrectly() { - ConfigurationDoneHandler configurationDoneHandler = new( - NullLoggerFactory.Instance, null, debugService, null, null, psesHost, workspace, null); - await configurationDoneHandler.LaunchScriptAsync(oddPathScriptFile.FilePath); + LaunchAndAttachHandler launchAndAttachHandler = new( + NullLoggerFactory.Instance, null, null, null, debugService, psesHost, psesHost, workspace, null, null); + await launchAndAttachHandler.LaunchScriptAsync(oddPathScriptFile.FilePath, [], "DotSource"); IReadOnlyList historyResult = await psesHost.ExecutePSCommandAsync( new PSCommand().AddScript("(Get-History).CommandLine"),