Powershell: RunspacePool mutates the PSModulePath

[rjmholt]

From https://github.com/PowerShell/vscode-powershell/issues/2008.

Related to https://github.com/PowerShell/PowerShell/issues/6850.

When using a RunspacePool (common in hosted and remote contexts), the PSModulePath gets reset whenever a runspace is opened.

This occurs because ModuleIntrinsics sets the module path at constructor time:

https://github.com/PowerShell/PowerShell/blob/66c628f1b0992312e66830a6f660e1fe99129863/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs#L41-L47

as if it were a process-singleton.

Every time a runspace is opened in PowerShell, the whole process gets its PSModulePath reset, which is a problem for things like EditorServices where we use a runspace to run analysis in the background, but users expect to be able to modify the PSModulePath in the IntegratedConsole.

[alx9r]

I just ran into this too. Here is a repro:

Repro

$userPath = '\some\path'
$ENV:PSModulePath = $ENV:PSModulePath+";$userPath"
$ENV:PSModulePath.Split(';')[-1] -eq $userPath
$runspace = [runspacefactory]::CreateRunspace()
$ENV:PSModulePath.Split(';')[-1] -eq $userPath
$runspace.Open()
$ENV:PSModulePath.Split(';')[-1] -eq $userPath

Expected Behavior

true
true
true

Actual Behavior

true
true
false

[alx9r]

@rjmholt Do you think I should expect that, in the long term, each runspace will have its own instance of PSModulePath? Or is there some competing interest that favors a single process-wide PSModulePath?

[rjmholt]

in the long term, each runspace will have its own instance of PSModulePath? Or is there some competing interest that favors a single process-wide PSModulePath?

That's a very good question, and one I think needs more design thought.

My thinking is:

• The PSModulePath has always been an env var and in PS 6+ can be configured in the powershell.config.json file. The first is process-wide, the second is PowerShell-install-wide (i.e. even wider, since all PowerShell processes started from that pwsh.exe will have that PSModulePath)
• Changing that would be a pretty big breaking change, so undesirable
• But as an env var, different runspaces shouldn't reset it -- it should be initialized once, possibly by the default runspace
• Runspace pool implementors should only mutate PSModulePath with care, and ideally just load modules directly anyway

[alx9r]

Those are all good points. I'll advocate for each runspace having its own PSModulePath for a bit:

• user code can mutate $PSModulePath
• such user code could be invoked from any runspace within the process
• when mutation of PSModulePath occurs in one runspace it affects the PSModulePath visible from other runspaces
• runspaces can run concurrently so errors resulting from the mutation will be difficult to reason about

This suggests to me that the process-wide PSModulePath singleton should continue to exist, but access to it from each runspace should be copy-on-write. That way each runspace sees the same, correct, initial value, but mutation from that runspace does not affect other runspaces.

[vexx32]

That does sound like the best solution, and would be mimicking PS's own scope behaviour of variables as well, so would generally be intuitively understood reasonably well I think.

[iSazonov]

The Issue will be resolver with #9957. In short PowerShell Committee conclusion is that PowerShell Core will do not modify $env:PSModulePath.

[alx9r]

@isazanov Can you elaborate on how #9957 will resolve this issue? #9957 seems to only contemplate the management of PSModulePath across process boundaries whereas this issue is about the management of PSModulePath across runspace boundaries within a process.

[alx9r]

I'm seeing this problem manifest in my implementation of Invoke-Parallel. For others facing this, here is a summary of the least-bad still-a-half-measure workaround I've found:

• Cache the value of PSModulePath prior to opening _any_ other runspaces (henceforth startingPSModulePath). Note that at least the Runspace, RunspacePool, and PowerShell classes can lead to opening a runspace, so it can be a bit of an undertaking to root out all such calls.
• Immediately overwrite PSModulePath with startingPSModulePath each time a runspace is opened.
• There will still be a (usually short) period when PSModulePath is in the reset state while the runspace is being opened, but before the overwrite has a chance to occur.
• If a call to Import-Module (or a second call to Runspace.Open() for that matter) coincides with that period then module importing can fail.
• The failed module importing manifests intermittently because it depends on the coincidental timing of opening runspaces and importing modules.
• This dramatically reduces the occurrence of failed module imports in multiple runspace scenarios which means development on such projects can continue. But such failures are still a statistical certainty with a likelihood that is probably higher than what most organizations can tolerate for production.

[iSazonov]

@alx9r My current plan is to implement a global cache s_psModulePath. We will only read $env:PSModulePath, not reset. I think it will resolve the issue and some others.
There can be an edge case - runspace needs to add/set a private path(s). I guess this is incredible. But if it appears, there are several ways to solve it.

[alx9r]

My current plan is to implement a global cache s_psModulePath. We will only read $env:PSModulePath, not reset. I think it will resolve the issue and some others.

@iSazonov OK I think understand now. Thank you.

There can be an edge case - runspace needs to add/set a private path(s). I guess this is incredible. But if it appears, there are several ways to solve it.

I expect that issue will appear. Currently $ENV:PSModulePath is mutable, so any user code could mutate it. I've certainly encountered such code. It will be very easy to unknowingly invoke such code in a multi-threaded runspace scenario. But I suppose that's a different issue from this one. :relaxed:

[msftbot[bot]]

:tada:This issue was addressed in #11057, which has now been successfully released as v7.0.0-preview.6.:tada:

Handy links:

• Release Notes

[rjmholt]

I'm not sure why this issue was closed in #11057, since the code still indicates that instantiating a new ModuleInstrinsics object resets the PSModulePath:

https://github.com/PowerShell/PowerShell/blob/0c46e3e71d628b96833da0299890be89fae430bf/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs#L42-L48

[daxian-dbw]

Does it reset it though? SetModulePath() is likely going to use the existing process scope PSModulePath value, not?

[iSazonov]

It's refresh PSModulePath based on current PSModulePath and computer and user scope configs.

[rjmholt]

I see the logic is more nuanced than I gave it credit for.

My worry though is that mutating the module path at all across runspaces creates a hidden race condition.

In the PowerShell extension, we embed PSScriptAnalyzer, which uses a runspace pool behind the scenes (and we can't pull it out). If a user sets the module path in their prompt, we have no way to guarantee that that gets used anywhere before it's tampered with by PowerShell.

Basically having something that constructs a new ModuleInstrinsics object makes PSModulePath unpredictable. On first startup it makes sense that we set the module path, but when a create a new runspace, I don't see any upsides to mutating the module path again. I also feel that we ought to not do it using a complex series of methods in a parameterless constructor.

[iSazonov]

@rjmholt That you ask can be resolved only if we do not write back to PSModulePath environment variable. I proposed the design and pulled PR but PowerShell Committee conclusion was "write back".

[daxian-dbw]

I do think PowerShell should only set the PSModulePath environment variable only once per process. The environment path mutation should probably be done in the static constructor of ModuleInstrinsics, but need to check back to the code to see if the current approach is "by design" somehow.

[iSazonov]

And after that we get feedback "I changed PSModulePath but PowerShell ignore this."
For me it is the same history as cwd - system and Core have a cwd but every runspace has their own cwd.
PSModulePath is used for module discovering and loading but PowerShell load modules per runspace so it should have $PSModulePath per runspace and use PSModulePath env only at startup and runspace initialization.

[daxian-dbw]

@iSazonov What you proposed was a too big breaking change that we cannot afford to accept.

[iSazonov]

@daxian-dbw I did not understand why this is "a too big breaking change" because no one indicated which applications will not work. It looks like a habit :-).
I see only one real scenario where we could populate PSModulePath env - every time we run subprocess with Start.Process() we can add actual PSModulePath env by ProcessInfo.

[msftbot[bot]]

This issue has been marked as fixed and has not had any activity for 1 day. It has been closed for housekeeping purposes.

[iSazonov]

The issue from vscode-powershell repo obviously says that we shouldn't write PSModulePath back, PowerShell must only read PSModulePath but not change.