Add support to ShellExecute in powershell core when it's running on Windows full SKUs#3281
Add support to ShellExecute in powershell core when it's running on Windows full SKUs#3281TravisEz13 merged 3 commits intoPowerShell:masterfrom
Conversation
| try | ||
| { | ||
| // Try Process.Start first. This works for executables even on headless SKUs. | ||
| invokeProcess.StartInfo.FileName = path; |
There was a problem hiding this comment.
Have you tested this on a headless SKU?
There was a problem hiding this comment.
Yes I did. It's able to start executable files, like .exe file, on NanoServer.
| $_.Exception.Message | Should match '-Verb' | ||
| } | ||
| ## -Verb is supported in PowerShell core on Windows full desktop. | ||
| It "Should give an error when -Verb parameter is used" -Skip:$isFullWin { |
There was a problem hiding this comment.
Should this test case be skipped on IoT and Nano Server?
There was a problem hiding this comment.
No, -Verb is not supported on non-windows platforms and windows headless SKUs. So, on NanoServer and IoT, using -Verb should result in a terminating error.
| } | ||
|
|
||
| ## -WindowStyle is supported in PowerShell core on Windows full desktop. | ||
| It "Should give an error when -WindowStyle parameter is used" -Skip:$isFullWin { |
There was a problem hiding this comment.
Same reason as above.
| $process | Stop-Process | ||
| } | ||
|
|
||
| It "Should open the application that associates with extension '.txt'" { |
There was a problem hiding this comment.
This will not run on IoT and Nano.
There was a problem hiding this comment.
Good catch. I will fix it.
| /// There are instances where ShellExecuteEx does not use one of these types of Shell extension and those instances would not require | ||
| /// COM to be initialized at all. Nonetheless, it is good practice to always initalize COM before using this function." | ||
| /// | ||
| /// TODO: In .NET Core, managed threads are all eagerly initialized with MTA mode, so to call 'ShellExecuteEx' from a STA thread, we |
There was a problem hiding this comment.
Do you have an issue number for this TODO?
There was a problem hiding this comment.
#2969 will be used to track the "invoke-on-STA-thread" work.
Yes, it's recorded in the description of this PR. This Start method will be touched when fixing that issue and this TODO can be removed then.
There was a problem hiding this comment.
I will add the issue number in the comment here as well.
| if (shellExecuteInfo.lpDirectory != (IntPtr)0) Marshal.FreeHGlobal(shellExecuteInfo.lpDirectory); | ||
| } | ||
|
|
||
| Process processToReturn = null; |
There was a problem hiding this comment.
Could you please write a comment on what is happening right here? Please correct me if I am wrong. In the finally block you are freeing memory for lpfile, lpVerb, lpParameters, and lpDirectory. Then you use the pointer to the process to fetch the process using its id. My question is: Do we need to free anymore memory from shellExecuteInfo once the process is returned?
There was a problem hiding this comment.
No more resource needs to be freed. This code mainly comes from .NET code here
| // If it's headless SKUs, rethrow. | ||
| if (Platform.IsNanoServer || Platform.IsIoT) { throw; } | ||
| // If it's full Windows, then try ShellExecute. | ||
| ShellExecuteHelper.Start(invokeProcess.StartInfo); |
There was a problem hiding this comment.
Is there a possibility that this call will throw? If it does, how are you handling it?
There was a problem hiding this comment.
If it throws, then the exception just propagates and powershell will handle it. It's like what would happen when invokeProcess.Start() throws.
|
Hi @daxian-dbw, I left some minor comments/questions. Overall it looks very good. Thanks for enabling this! |
|
@Francisco-Gamino I have addressed your comments. Could you please take another look? |
|
@daxian-dbw: LGTM, thanks for getting this done! |
Add support to ShellExecute in powershell core when it's running on Windows full SKUs.
Updated
Get-Help -Online,Invoke-ItemandStart-Processto support start a process using ShellExecute on powershell core on windows full SKUs.TODO: In .NET Core, managed threads are all eagerly initialized with MTA mode, so to call
ShellExecuteExfrom a STA thread, we need to create a native thread usingCreateThreadfunction and initialize COM with STA on that thread. In #3281, we are callingShellExecuteExdirectly 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. #2969 will be used to track the "invoke-on-STA-thread" work.