GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Make 'Start-Job' throw terminating error when PowerShell is being hosted by daxian-dbw · Pull Request #9128 · PowerShell/PowerShell

Conversation

@daxian-dbw

@daxian-dbw Dongbo Wang (daxian-dbw) commented Mar 12, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

PR Summary

Related: #7052

The pwsh executable is not available under $PSHOME in the scenarios where PowerShell is being hosted in other applications, like PowerShell Azure Functions.
In those cases, Start-Job won't work as there is no pwsh for us to reliably start a out-of-proc Runspace. So make Start-Job throw a terminating error in such cases and recommend ThreadJob module.

Also suppressed -HostName and -SubSystem parameters that are incorrectly exposed by Start-Job.

PR Context

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Change is not breaking
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
  • User-facing changes
    • Not Applicable
    • OR
    • Documentation needed
      • Issue filed:
  • Testing - New and feature
    • N/A or can only be tested interactively
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed
      • Add [feature] to your commit messages if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@daxian-dbw
Make 'Start-Job' throw terminating error when PowerShell is being hosted
65baccc
@daxian-dbw
Dongbo Wang (daxian-dbw) requested review from Bruce Payette (BrucePay), Paul Higinbotham (PaulHigin) and Mike Richmond (mirichmo) as code owners March 12, 2019 22:35
@daxian-dbw
Dongbo Wang (daxian-dbw) requested review from Steve Lee (SteveL-MSFT) and TylerLeonhardt and removed request for Bruce Payette (BrucePay) and Mike Richmond (mirichmo) March 12, 2019 22:36
@daxian-dbw
Add xUnit test
3ce785d
@daxian-dbw Dongbo Wang (daxian-dbw) assigned Aditya Patwardhan (adityapatwardhan) Mar 12, 2019
SteveL-MSFT
Steve Lee (SteveL-MSFT) reviewed Mar 12, 2019
View reviewed changes
Comment thread src/System.Management.Automation/engine/hostifaces/PowerShellProcessInstance.cs

if ((!string.IsNullOrEmpty(procArch)) && (procArch.Equals("amd64", StringComparison.OrdinalIgnoreCase) ||
procArch.Equals("ia64", StringComparison.OrdinalIgnoreCase)))
if (!string.IsNullOrEmpty(procArch) && (procArch.Equals("amd64", StringComparison.OrdinalIgnoreCase) ||

@SteveL-MSFT Steve Lee (SteveL-MSFT) Mar 12, 2019

Copy link
Copy Markdown
Member

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.

Aditya Patwardhan (@adityapatwardhan) does this need to be updated for ARM?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Mar 12, 2019

Copy link
Copy Markdown
Member

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.

Yes, but what is the expected behavior on using useWow64 when run on arm64?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Mar 12, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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.

I don't think we should support the -RunAs32 parameter on Start-Job anymore. To start a 32-bit process pwsh, it requires the win-32 pwsh package to be installed, and there is no way to guarantee that, and I don't think we should even bother to search for that installation.

If you look at the current implementation closely, you will find that when useWow64 is true, we are actually using the same default pwsh under $PSHOME. So it's already broken. No one reports it because no one is using it.

I think when -RunAs32 is specified in Start-Job, we check if the current process is 64-bit, if so, we throw terminating error. If not, then we continue to run because the underlying pwsh is a 32-bit executable anyways. But this should be done by a separate PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Mar 13, 2019

Copy link
Copy Markdown
Member

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.

I agree that the -RunAs32 is probably not used much anymore since the world has moved to 64-bit and makes even less sense for pwsh

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Mar 13, 2019

Copy link
Copy Markdown
Member

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.

We can do that in a separate PR and not block this one

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Mar 13, 2019

Copy link
Copy Markdown
Member Author

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.

Agreed. I will submit a separate PR for that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
TylerLeonhardt
TylerLeonhardt approved these changes Mar 12, 2019
View reviewed changes

@TylerLeonhardt TylerLeonhardt left a comment

Copy link
Copy Markdown
Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
iSazonov
Ilya (iSazonov) reviewed Mar 13, 2019
View reviewed changes
Comment thread test/xUnit/csharp/test_PowerShellAPI.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@daxian-dbw
[Feature] Fix Codacy issue
e08f123
iSazonov
Ilya (iSazonov) approved these changes Mar 13, 2019
View reviewed changes
adityapatwardhan
Aditya Patwardhan (adityapatwardhan) approved these changes Mar 14, 2019
View reviewed changes
@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) merged commit cdb28a1 into PowerShell:master Mar 14, 2019
@daxian-dbw
Dongbo Wang (daxian-dbw) deleted the error branch March 14, 2019 01:06
@daxian-dbw Dongbo Wang (daxian-dbw) mentioned this pull request Mar 15, 2019
Documentation about 'Start-Job' should be updated MicrosoftDocs/PowerShell-Docs#4003
Closed
2 of 8 tasks
@iSazonov Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Mar 16, 2019
@TravisEz13 Travis Plunk (TravisEz13) added this to the 6.2.0-GA-Consider milestone Mar 18, 2019
Travis Plunk (TravisEz13) pushed a commit that referenced this pull request Mar 21, 2019
@daxian-dbw @TravisEz13
Make 'Start-Job' throw terminating error when PowerShell is being hos…
775a2eb 
…ted (#9128)
@TravisEz13 Travis Plunk (TravisEz13) modified the milestones: 6.2.0-GA-Consider, 6.2.0 Mar 21, 2019
This was referenced Mar 28, 2019
Update the doc for Start-Job MicrosoftDocs/PowerShell-Docs#4068
Merged
PSJobs - Unable to receive jobs Azure/azure-functions-powershell-worker#181
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@daxian-dbw @adityapatwardhan
Make 'Start-Job' throw terminating error when PowerShell is being hos…
d4c69db 
…ted (PowerShell#9128)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT left review comments
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) adityapatwardhan approved these changes
@iSazonov Ilya (iSazonov) iSazonov approved these changes
@PaulHigin Paul Higinbotham (PaulHigin) Awaiting requested review from PaulHigin
+1 more reviewer
@TylerLeonhardt TylerLeonhardt TylerLeonhardt approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@adityapatwardhan Aditya Patwardhan (adityapatwardhan)

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

6.2.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@daxian-dbw @TylerLeonhardt @SteveL-MSFT @adityapatwardhan @iSazonov @TravisEz13
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.