FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix PowerShell class definition leaking into another Runspace by daxian-dbw · Pull Request #11273 · PowerShell/PowerShell · GitHub

Fix PowerShell class definition leaking into another Runspace - #11273

Merged
Aditya Patwardhan (adityapatwardhan) merged 1 commit into
PowerShell:masterfrom
daxian-dbw:leak
Dec 11, 2019
Merged

Fix PowerShell class definition leaking into another Runspace#11273
Aditya Patwardhan (adityapatwardhan) merged 1 commit into
PowerShell:masterfrom
daxian-dbw:leak

Conversation

Dongbo Wang (daxian-dbw) commented Dec 5, 2019
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

Fix #9313

The root problem is that a scope is holding on the shared default TypeResolutionState object when there is no PowerShell class defined in the parent chain of the scope, and then the TypeResolutionState object is used as a key to cache the found type.
A scope should not hold on to the default TypeResolutionState object, but should always refer to the parent's TypeResolutionState object when no PowerShell class defined in itself. When a PowerShell class is defined in a scope, a new TypeResolutionState will be created for the specific scope.

PR Context

PR Checklist

Dongbo Wang (daxian-dbw) added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Dec 5, 2019

Copy link
Copy Markdown
Member

Rob Holt (@rjmholt) Can you review?

Copy link
Copy Markdown
Collaborator

What is a design intention - runspaces is boundaries of visibility of c# types, ps classes, objects, modules?
If before the PR I define a ps class in interactive session and run a job (or foreach -parallel) then after the change the job will does not see the class?

Dongbo Wang (daxian-dbw) commented Dec 10, 2019
edited
Loading

Copy link
Copy Markdown
Member Author

PowerShell class is visible only in the scope where it's defined and the child scopes of that scope. It's a scope thing, just like you define a function.

Copy link
Copy Markdown
Member Author

Actually, Ilya (@iSazonov) brings up a good point and makes me believe this should be included in rc.1. If people started to depend on the wrong behavior, it will be much worse to break it in 7.1.0.

Copy link
Copy Markdown

🎉v7.0.0-rc.1 has been released which incorporates this pull request.:tada:

Handy links:

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PowerShell classes leak to other runspaces on macOS

6 participants


Back | FazBrowse Home | New Git URL