-
Notifications
You must be signed in to change notification settings - Fork 234
Workaround "attach to process" hang #846
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Good catch on needing the fully qualified type name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! One suggestion
// Attaching to process currently needs to be marked as a local session | ||
// so we skip this if block if the runspace is from Enter-PSHostProcess | ||
if (hostName.Equals("ServerRemoteHost", StringComparison.Ordinal) | ||
&& runspace.OriginalConnectionInfo.GetType().ToString() != "System.Management.Automation.Runspaces.NamedPipeConnectionInfo") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be a good idea to do a null check here. I think there are a couple ConnectionInfo
's that do not get written here but are still ServerRemoteHost
&& runspace.OriginalConnectionInfo.GetType().ToString() != "System.Management.Automation.Runspaces.NamedPipeConnectionInfo") | |
&& runspace.OriginalConnectionInfo != null | |
&& runspace.OriginalConnectionInfo.GetType().ToString() != "System.Management.Automation.Runspaces.NamedPipeConnectionInfo") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better yet:
runspace.OriginalConnectionInfo?.GetType().ToString() != "System.Management.Automation.Runspaces.NamedPipeConnectionInfo"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops! Got stuck in bool mode and had it in my head you'd have to do ?? false
. Nice catch :)
This fixes the "Attach to process" debug config. Originally broken in #801.
Unfortunately, I'm not 100% sure why the Enter-PSHostProcess session being treated as a Remote session causes issues but this code was majorly refactored in 2.0.0 branch so I'm just going to make this small change for now.
fixes PowerShell/vscode-powershell#1684