PowerShell / PowerShellEditorServices

A common platform for PowerShell development support in any editor or application!
MIT License
625 stars 215 forks source link

Dead lock when getting dynamic parameters for `Install-Package` while PSRL is running #762

Closed SeeminglyScience closed 2 years ago

SeeminglyScience commented 5 years ago

I've been looking at what PowerShell says is running every time I've hit a dead lock. Every time, it's Install-Package generating dynamic parameters.

Repro

  1. Install module ImpliedReflection (or other preferred reflection module)

  2. Wait for a dead lock after completion

  3. Get the process ID of the integrated console with Get-PSHostProcess (Hint: it won't have a window title)

  4. Enter the process with Enter-PSHostProcess

  5. Get the main PSIC runspace with $rs = Get-Runspace 2

  6. Enable implied reflection with Enable-ImpliedReflection

  7. Navigate to $rs.ExecutionContext.CurrentCommandProcessor. Should look something like this:

    Click to expand output ```raw FromScriptFile : False _cmdletParameterBinderController : System.Management.Automation.CmdletParameterBinderController u_obsoleteAttribute : _firstCallToRead : True _bailInNextCall : False RanBeginAlready : False _addedToPipelineAlready : False _fromScriptFile : False CommandRuntime : Install-Package _useLocalScope : True _context : System.Management.Automation.ExecutionContext arguments : {} CmdletParameterBinderController : System.Management.Automation.CmdletParameterBinderController ObsoleteAttribute : AddedToPipelineAlready : False CommandInfo : Install-Package RedirectShellErrorOutputPipe : False Command : Microsoft.PowerShell.PackageManagement.Cmdlets.InstallPackage UseLocalScope : True Context : System.Management.Automation.ExecutionContext PipelineActivityId : 00000000-0000-0000-0000-000000000000 CommandSessionState : System.Management.Automation.SessionStateInternal CommandScope : System.Management.Automation.SessionStateScope ```
  8. Navigate to $rs.ExecutionContext.CurrentCommandProcessor.Command. Should look something like this:

    Click to expand output ```raw InputObject : Name : RequiredVersion : MinimumVersion : MaximumVersion : Source : Credential : Proxy : ProxyCredential : Sources : {} WebProxy : CredentialUsername : CredentialPassword : AllVersions : False ProviderName : MessageResolver : DynamicOptions : {} OptionKeys : {} IsInteractive : False CallCount : 2 WhatIf : False IsCanceled : False DynamicParameterDictionary : {} ParameterSetName : PackageBySearch MyInvocation : System.Management.Automation.InvocationInfo PagingParameters : InvokeCommand : System.Management.Automation.CommandInvocationIntrinsics Host : System.Management.Automation.Internal.Host.InternalHost SessionState : System.Management.Automation.SessionState Events : System.Management.Automation.PSLocalEventManager JobRepository : System.Management.Automation.JobRepository JobManager : System.Management.Automation.JobManager InvokeProvider : System.Management.Automation.ProviderIntrinsics Stopping : False CommandRuntime : Install-Package CurrentPSTransaction : CommandOrigin : Internal Force : False ForceBootstrap : False k__BackingField : k__BackingField : k__BackingField : k__BackingField : k__BackingField : _resultsPerName : {} _providersNotFindingAnything : {} IsFailingEarly : False UserSpecifiedSourcesList : {} CmdletState : GenerateParameters CancellationEvent : System.Threading.CancellationTokenSource ErrorState : False currentObjectInPipeline : CommandOriginInternal : Internal ParameterSets : {PackageBySearch, PackageByInputObject} SpecifiedMinimumOrMaximum : False BootstrapNuGet : true SelectedProviders : {Programs, msu, msi, NuGet...} CachedSelectedProviders : {Programs, msu, msi, NuGet...} CachedDynamicOptions : {IncludeWindowsInstaller, IncludeSystemComponent, AdditionalArguments, ConfigFile...} CachedStaticParameters : IsDisplayCulture : False UseDefaultSourceFormat : True ShouldLogError : True ShouldSelectAllProviders : False IsPackageBySearch : True IsPackageByObject : False IsSourceByObject : False PackageManagementService : Microsoft.PackageManagement.Internal.Implementation.PackageManagementService TimeOut : 3600 Responsiveness : 900 PackageManagementHost : IHostApi_proxy_20 IsReentrantLocked : True Confirm : False IsInvocation : False HasErrors : False UnboundArguments : {} IsProcessing : False IsBeforeProcessing : True IsAfterProcessing : False HasDynamicParameters : True RunspaceRepository : System.Management.Automation.RunspaceRepository _ParameterSetName : PackageBySearch InvocationExtent : CurrentPipelineObject : PSHostInternal : System.Management.Automation.Internal.Host.InternalHost InternalState : System.Management.Automation.SessionState IsStopping : False CommandInfo : Install-Package Context : System.Management.Automation.ExecutionContext ```

PackageManagement makes heavy use of async in their cmdlets. My guess is they are inheriting our single threaded background synchronization context when they previously were not.

Note: Cancelling the command via their internal API ($rs.ExecutionContext.CurrentCommandProcessor.Command.Cancel()) doesn't do anything, which makes sense if it's a threading related dead lock.

SeeminglyScience commented 5 years ago

This is a bit more complicated than I initially expected.

Series of events

  1. PSReadLine is running in the main pipeline thread
  2. In the editor, some portion of the cmdlet Install-Package is typed
  3. A completion request is triggered for the typed text
  4. We take over the pipeline thread via an OnIdle event subscriber
  5. The GetDynamicParameters method is called on Install-Package during CompleteInput
  6. Install-Package enters a new thread
  7. Install-Package calls MyInvocation.MyCommand.Parameters
  8. The getter for the Parameters property then creates an event to get back onto the pipeline thread
  9. Dead lock

Gathered points of interest

rjmholt commented 5 years ago

Tagging @daxian-dbw, @PaulHigin, @BrucePay and @lzybkr, since we are in need of some expertise of PowerShell's threaded behaviour. Essentially our usage of PSReadLine is resulting in a deadlock with the PackageManagement cmdlets inside PowerShell, and we're not entirely sure how to get around it.

I've read over the possibly related https://github.com/PowerShell/PowerShell/issues/4003, but ultimately we need this to work in Windows PowerShell as well.

We'd be very appreciative of any advice you can offer. I'm sure we can give proper explanations of PSES' implementation wherever needed (good exercise for us anyway).

lzybkr commented 5 years ago

If PSReadLine is running, the PowerShell engine is "busy" because the function PSConsoleReadLine is running.

In the early PSReadLine days, folks noticed their OnIdle events weren't processed until after PSReadLine finished.

This is currently broken in 2.0, see https://github.com/lzybkr/PSReadLine/issues/773.

I don't know when this regressed (no regression tests for this unfortunately), but the VSCode/PSReadLine is a likely candidate to have caused the regression.

I'd start with fixing https://github.com/lzybkr/PSReadLine/issues/773 and maybe this deadlock will be fixed.

SeeminglyScience commented 5 years ago

@lzybkr The OnIdle event is definitely working, the dead lock happens while CommandCompletion.CompleteInput() is running after already taking the thread from PSReadLine. Also this is only an issue when Install-Package (and most likely other cmdlets from PackageManagement) are a part of the completion results.

I believe the problem is that Install-Package puts itself on another thread to generate parameters, then accesses a property that tries to put itself back onto the pipeline thread with an event. But the first event (for command completion) is still running.

lzybkr commented 5 years ago

OK, I hadn't looked at the OnIdle issue yet, but from your description, it sounds like the problem would reproduce without PSReadLine. Maybe @jianyunt has some insight here.

jianyunt commented 5 years ago

PackageManagement indeed is using async and dynamics a lot. GetDynamicParameters is called here: https://github.com/OneGet/oneget/blob/173df840e345d0a753dd0a9dec684e394f1b22ea/src/Microsoft.PowerShell.PackageManagement/Cmdlets/CmdletWithProvider.cs#L426

But we have not heard deadlock from other users though.

SeeminglyScience commented 5 years ago

@jianyunt Yeah it's not a particularly common scenario so I'm not surprised it hasn't come up. Here's an easy way to reproduce it outside of VSCode:

$splat = @{
    SourceIdentifer = [System.Management.Automation.PSEngineEvent]::OnIdle
    Action = { (Get-Command Install-Package).Parameters }
}

Register-EngineEvent @splat 

If you run that the prompt will become unresponsive.

I've fixed the issue by saving MyInvocation.MyCommand.Parameters prior to calling AsyncRun, that clears up the deadlock. I can submit a PR after some more testing.

jianyunt commented 5 years ago

👍

SeeminglyScience commented 2 years ago

This has since been solved in a few ways on both ends (we don't use the onidle engine event, and oneget is fixed). Closing.