Skip to content
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

Hang (possible deadlock) in PSScriptAnalyzer #1297

Closed
rjmholt opened this issue Jul 27, 2019 · 10 comments · Fixed by #1316
Closed

Hang (possible deadlock) in PSScriptAnalyzer #1297

rjmholt opened this issue Jul 27, 2019 · 10 comments · Fixed by #1316

Comments

@rjmholt
Copy link
Contributor

rjmholt commented Jul 27, 2019

Steps to reproduce

Warning: Do this in a new process that you know the PID of -- it will lock up.

iwr https://raw.githubusercontent.com/TravisEz13/PsAzDevOpsExt/master/src/PSPackageProject.psm1 -outfile ex.ps1
Invoke-ScriptAnalyzer -Path ex.ps1

Expected behavior

PSSA emits any relevant diagnostics and returns

Actual behavior

Call never returns and does not respond to Ctrl+C

Environment data

> $PSVersionTable
                                                                                   Name                           Value                                               ----                           -----                                               PSVersion                      7.0.0-preview.2
PSEdition                      Core
GitCommitId                    7.0.0-preview.2
OS                             Darwin 18.7.0 Darwin Kernel Version 18.7.0: Thu Ju…
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.18.1

Debugging this, it seems to occur around this call. This behaviour is not present in 1.18.0.

@rjmholt
Copy link
Contributor Author

rjmholt commented Jul 27, 2019

/cc @JamesWTruher

@rjmholt
Copy link
Contributor Author

rjmholt commented Jul 27, 2019

The suspicion is that this is caused by the runspace pool deadlocking. The immediate solution is to rip that out, and down the line we should investigate implementing our own runspace pool.

@bergmeister
Copy link
Collaborator

Possibly related: #1287

@rjmholt
Copy link
Contributor Author

rjmholt commented Jul 27, 2019

The next step I think is to get a proper stack trace on Windows using a debugging environment that can drill into the platform code; it would be nice to prove that this is a deadlock

@rjmholt
Copy link
Contributor Author

rjmholt commented Jul 30, 2019

Ok, after a bit more analysis I believe this is the same issue as @SeeminglyScience describes in PowerShell/PowerShellEditorServices#762 (comment)

@rjmholt
Copy link
Contributor Author

rjmholt commented Jul 30, 2019

Notice that the repro script contains a reference to Install-Package, and a few other PackageManagement cmdlets

@rjmholt
Copy link
Contributor Author

rjmholt commented Jul 30, 2019

pssa.log

windbg thread stack dump

@SeeminglyScience
Copy link
Contributor

SeeminglyScience commented Jul 31, 2019

Looking at the windbg dump, I agree that definitely looks like the same issue. I'm not sure if there is anything on PSSA's end that can be done though.

I got side tracked before I could submit a PR to oneget, but I've been running with these changes for a few months now without any issues. Not sure when I'll get to submitting that PR if someone else wants to take that on. (Note that the fix itself could use some refinement, if possible avoiding GetAwaiter().GetResult())

@rjmholt
Copy link
Contributor Author

rjmholt commented Jul 31, 2019

Unfortunately I think even if we get those changes into PackageManagement (which we should try to do), it will be hard to propagate them to users. Similarly, I think it's worth investigating if something can be done in PowerShell to prevent this, but it will only be possible to make such a change in PS 7.

So I think PSSA and PSES will need to special case the PackageManagement cmdlet parameters. Not ideal, but not all that hard. And they're not going to change, especially with PSGet 3.0 coming.

@SeeminglyScience
Copy link
Contributor

SeeminglyScience commented Jul 31, 2019

So I think PSSA and PSES will need to special case the PackageManagement cmdlet parameters. Not ideal, but not all that hard. And they're not going to change, especially with PSGet 3.0 coming.

Ah right PSSA can do that, I forgot it's the one calling CommandInfo.Parameters. I can't think of a way that PSES could though, it's being triggered through TabExpansion2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants