-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Fix using variable for nested foreach parallel calls #14548
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
Fix using variable for nested foreach parallel calls #14548
Conversation
|
@daxian-dbw Can you please review these changes? |
SteveL-MSFT
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment, otherwise looks fine to me
src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs
Outdated
Show resolved
Hide resolved
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
|
Restarted CIs for what looks like unrelated failures in Update-Help tests. |
|
@PaulHigin it would be good to fix CodeFactor violations in the new code that is added; thank you. |
|
Restarted CIs again. |
|
One more time restarted CIs. |
|
🎉 Handy links: |
|
@PaulHigin - Should this be backported to 7.1.x? |
|
@adityapatwardhan I think the impact of this change is small, since if involves nested foreach calls. So I would say it does not need to be backported as it probably does not affect a lot of users. |
|
Removed the back port label as per comment from @PaulHigin |
|
This is not unique to parallel foreach, it applies to nested start-job and nested invoke-command calls (double-hop) or any combination of those, like using invoke-command and then running start-job in the remote session, etc. Does this fix work for these scenarios as well (everywhere $using is supported) or was this somehow scoped to parallel foreach? |
PR Summary
This fixes a bug in ForEach-Object -Parallel where a using variable in a nested ForEach-Object -Parallel throws an error even when the variable is defined in the correct scope (Issue #11817).
PR Context
This error was occurring because, when ForEach-Object -Parallel was assembling the user variable map, it was searching all nested scriptblocks within the ForEach scriptblock, with the result of finding nested using variables where the variable had not yet been defined. Since the nested scriptblock using variable had not been defined in the current scope, a mapping error was thrown.
Fix is to change the using variable map function to not search nested scriptblocks in the ForEach -Parallel case. This way foreach -parallel using variable mapping is always performed only for the current scope.
Many thanks to @mklement0 for pointing out the fix.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.