-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add APIs for breakpoint management in runspaces and enable attach to process without BreakAll for PSES #10338
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
Add APIs for breakpoint management in runspaces and enable attach to process without BreakAll for PSES #10338
Conversation
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
|
Looks like there are some Pester test failures that have nothing to do with this PR. |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
|
@PaulHigin: Regarding the intermittent hang, my suspicion is that there is a timing issue with the initial handshake in MS-PSRDP, where the initial outgoing message from the client fires before the server starts actively listening for requests, at which point both client and server are waiting for communication from one another: the client waits for the server to respond, and the server waits for the client to tell it what to do. I believe this is an existing issue, not new, and it can probably be reproduced with the original set of APIs that MS-PSRDP supports. |
|
@PaulHigin Thinking about this more, and reading through the code this morning, I'm convinced that the scenario I shared above is the root cause, and the issue I tripped on is related to issue #8012. Neither This issue is not related to the changes in this PR, but this PR and the original test suite written to validate these changes exposed the issue. That should be handled separately, as an RFC according to the committee review of #8012, which should propose a solution for both In the meantime, I just changed the Pester tests that use the new MS-PSRDP APIs such that they won't run until the runspace is truly ready. The easiest workaround I could come up with was to add a breakpoint and wait for the runspace to hit that breakpoint before using the APIs. Making this change allows me to run the tests over and over and over, and it never hangs. I'll commit that change in just a moment. |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/remoting/client/remoterunspace.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/remoting/client/remoterunspace.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/remoting/client/remoterunspace.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/DebugRunspaceCommand.cs
Show resolved
Hide resolved
|
@KirkMunro, I'll add you (and @vexx32) to the approved list. I should probably also allow |
src/System.Management.Automation/engine/remoting/client/remoterunspace.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/remoting/client/remoterunspace.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/remoting/server/ServerRunspacePoolDriver.cs
Outdated
Show resolved
Hide resolved
|
FYI, the Codacy issue that is being flagged is a false positive. |
|
Any chance we can get some progress on the pending reviews here? As indicated, the Codacy issue is a false positive. This PR is good to go, just needs reviews finalized so that it can be merged. Other work is pending the merge of this PR. |
|
@KirkMunro Thank you for your contribution! |
…process without BreakAll for PSES (PowerShell#10338)
|
🎉 Handy links: |
PR Summary
New-PSBreakpointexperimental cmdlet. This functionality is provided via SDK commands for now, and will be provided via a-Runspaceparameter on*-PSBreakpointcommands in a PR once this one is finalized.SetCommandBreakpoint,SetVariableBreakpoint,SetLineBreakpoint,EnableBreakpoint,DisableBreakpoint, andRemoveBreakpointpublic APIs to theDebuggerclass, plumbs them through the engine, and hooks them up for remote debugger work. Also hooks up theGetBreakpoint,GetBreakpoints, andSetBreakpointsDebuggerclass APIs for remote debugger work.-NoInitialBreakexperimental parameter toDebug-RunspaceandDebug-Jobthat allows users to attach the debugger without invoking aBreakAll. The corresponding methods for the same have boolean parameters allowing this as well.Set-PSBreakpointto the new debugger APIs that have superceded the ones it used to use.PR Context
This PR is needed for "attach to process" debugging in PowerShell Editor Services.
It helps because:
Debug-*to attach the debugger to that runspace.BreakAllin PSES.Important details about this PR:
-NoInitialBreakparameter added toDebug-RunspaceandDebug-Job, nor does it include Pester tests for the corresponding boolean value in theDebugRunspaceandDebugJobAPIs. I'm not sure how to automate that test, and believe it should be tested manually. Fortunately, that is an easy manual test to take on.Breakpoint.Tests.ps1file sometimes hang. I don't know why this happens. It has to do with working with a remote runspace and using the breakpoint APIs against it. It doesn't happen all the time, but it happens often. After 30 minutes, it will time out and fail. I need help identifying why this happens, by someone who understands the MS-PSRDP better than I do.cc: @TylerLeonhardt @PaulHigin @SteveL-MSFT
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.