Skip to content

Hang (possible deadlock) in PSScriptAnalyzer #1297

Closed
#1316
Closed
Hang (possible deadlock) in PSScriptAnalyzer#1297
#1316

Description

@rjmholt
Rob Holt (rjmholt)
opened on 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.

Activity

  1. rjmholt commented on Jul 27, 2019

    @rjmholt
    Rob Holt (rjmholt)
    on Jul 27, 2019
    ContributorAuthor

    /cc James Truher (@JamesWTruher)

  2. rjmholt commented on Jul 27, 2019

    @rjmholt
    Rob Holt (rjmholt)
    on Jul 27, 2019
    ContributorAuthor

    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.

  3. bergmeister commented on Jul 27, 2019

    @bergmeister
    Christoph Bergmeister (bergmeister)
    on Jul 27, 2019
    Collaborator

    Possibly related: #1287

  4. rjmholt commented on Jul 27, 2019

    @rjmholt
    Rob Holt (rjmholt)
    on Jul 27, 2019
    ContributorAuthor

    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

  5. rjmholt commented on Jul 30, 2019

    @rjmholt
    Rob Holt (rjmholt)
    on Jul 30, 2019
    ContributorAuthor

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

  6. rjmholt commented on Jul 30, 2019

    @rjmholt
    Rob Holt (rjmholt)
    on Jul 30, 2019
    ContributorAuthor

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

  7. rjmholt commented on Jul 30, 2019

    @rjmholt
    Rob Holt (rjmholt)
    on Jul 30, 2019
    ContributorAuthor

    pssa.log

    windbg thread stack dump

  8. SeeminglyScience commented on Jul 31, 2019

    @SeeminglyScience
    Patrick Meinecke (SeeminglyScience)
    on Jul 31, 2019
    Collaborator

    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())

  9. rjmholt commented on Jul 31, 2019

    @rjmholt
    Rob Holt (rjmholt)
    on Jul 31, 2019
    ContributorAuthor

    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.

  10. SeeminglyScience commented on Jul 31, 2019

    @SeeminglyScience
    Patrick Meinecke (SeeminglyScience)
    on Jul 31, 2019
    Collaborator

    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.

  11. James Truher (JamesWTruher)
    closed this as completedin #1316on Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions