PowerShell / PowerShell

PowerShell for every system!
https://microsoft.com/PowerShell
MIT License
45.65k stars 7.31k forks source link

powershell should run `ShellExecuteEx` on STA thread #2969

Closed joeyaiello closed 7 years ago

joeyaiello commented 7 years ago

[Edited by @daxian-dbw]

PR #3281 adds support to ShellExecute in powershell core when it's running on Windows full SKUs. With that fix, Start-Process is able to run MSI in powershell core.

However, it's not a complete solution. It calls ShellExecuteEx directly in MTA thread instead of from STA thread as suggested in MSDN. Therefore, the current solution may not work for some shell extensions that require COM.

In .NET Core, managed threads are all eagerly initialized with MTA mode, so to call ShellExecuteEx from a STA thread, we need to create a native thread using CreateThread function and initialize COM with STA on that thread. In #3281, we are calling ShellExecuteEx directly on MTA thread, and it works for things like opening a folder in File Explorer, opening a file with the application that is associated with its extension in shell, opening URL in web browser and etc, but it's not guaranteed to work in all ShellExecution scenarios.

Will keep this issue open to track the "invoke-on-STA-thread" work.


PS:4> Start-Process .\PowerShell_6.0.0-alpha.17-win10-win2016-x64.msi -PassThru

 NPM(K)    PM(M)      WS(M)     CPU(s)     Id  SI ProcessName
 ------    -----      -----     ------     --  -- -----------
      9     6.15       7.55       0.03  20092   3 msiexec

PS:5> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      6.0.0-alpha
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
PSEdition                      Core
GitCommitId                    v6.0.0-alpha.16-54-gc86a287726fe366895723dfcc7739f93038615ea
CLRVersion
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
SerializationVersion           1.1.0.1
BuildVersion                   3.0.0.0
iSazonov commented 7 years ago

I test this for .txt - it don't work too. Seems windows type associations don't work here at all.

mattpwhite commented 7 years ago

The .NET Core version of System.Diagnostics.Process doesn't support using ShellExecute() to start processes. The .NET Framework version does and does so by default, with an alternate path that uses CreateProcess(). ShellExecute is what makes starting a file launch its associated executable (and other stuff like starting a URL launching your default browser). Have to imagine this is a breaking change for a lot of existing scripts (and that it's probably not trivial to fix).

https://referencesource.microsoft.com/#System/services/monitoring/system/diagnosticts/ProcessStartInfo.cs,48

https://github.com/dotnet/corefx/blob/912cba3675a15718515417e7e9b46504c0303fbc/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.Windows.cs#L85

daxian-dbw commented 7 years ago

3281 is to add ShellExecute support in powershell core.

It's not a complete solution however. It calls ShellExecuteEx directly in MTA thread instead of from STA thread as suggested in MSDN. Therefore, the current solution may not work for some shell extensions that require COM.

In .NET Core, managed threads are all eagerly initialized with MTA mode, so to call ShellExecuteEx from a STA thread, we need to create a native thread using CreateThread function and initialize COM with STA on that thread. In #3281, we are calling ShellExecuteEx directly on MTA thread, and it works for things like opening a folder in File Explorer, opening a file with the application that is associated with its extension in shell, opening URL in web browser and etc, but it's not guaranteed to work in all ShellExecution scenarios.

I will keep this issue open even after merging that PR to track the "invoke-on-STA-thread" work.

iSazonov commented 7 years ago

CoreFX issue https://github.com/dotnet/corefx/issues/522 milestone is 1.0.0-rtm so I set "Waiting - NetStandard20"

joeyaiello commented 7 years ago

@iSazonov So I'll be the first to admit that this thread is totally over my head, but 1.0.0-rtm in the CoreFX repo implies that that work already shipped in .NET Core 1.0 RTM.

Is .NET Standard 2.0 still relevant here?

iSazonov commented 7 years ago

I can't still find full support STA in CoreFX 😕

daxian-dbw commented 7 years ago

The CoreFX issue was not addressed. It was closed as I quoted here:

We should consider this when we have a concrete motivating scenario that requires it.

jeffbi commented 7 years ago

If it helps motivate them, without STA we cannot use IGroupPolicyObject (#3353)

daxian-dbw commented 7 years ago

@jeffbi Please leave a comment in the CoreFX issue. Maybe even reactivate the issue if you think it's a concrete scenario that should be supported in .NET Core.

jeffbi commented 7 years ago

@daxian-dbw I have added a comment on the CoreFX issue, but I don't think I can reopen it.

joeyaiello commented 7 years ago

@jeffbi it's probably better to open a new issue, explain the scenario for our purposes, and then list references to this issue and the original closed issue. Sound okay?

SteveL-MSFT commented 7 years ago

STA is now in corefx

daxian-dbw commented 7 years ago

Sweet! We need to clean up ApartmentState related workarounds in powershell now.