Runtime: Light up Process.UseShellExecute when available on Windows

Created on 25 Apr 2017  路  8Comments  路  Source: dotnet/runtime

It's on by default in Desktop and always off for Windows in Core: https://github.com/dotnet/corefx/issues/10361 https://github.com/dotnet/corefx/issues/4794
It's has an analogous implementation on Unix https://github.com/dotnet/corefx/pull/4861

The reason it's not in Core is ShellExecuteEx is not available in all OneCore platforms. Also " ShellExecuteEx needs to run as STA but managed code runs as MTA by default and Thread.SetApartmentState() is not supported on all platforms."

It's a heavily used API: https://apisof.net/catalog/System.Diagnostics.ProcessStartInfo.UseShellExecute eg up to 25% in Unity apps. It's also part of .NET Standard 2.0. So we should consider whether there's a way to light up at lesat part of its functionality when (in the 99% case) we do have ShellExecuteEx available.

I'm not sure whether we have any precedent in Core for attempting to use a non-OneCore API.

Post 2.0

@stephentoub

area-System.Diagnostics.Process enhancement

Most helpful comment

This is done and ported into 2.0 馃憤

All 8 comments

Just as more "anecdata", I've seen quite a few folks trying to use this API and being confused about the difference in gitter, stackoverflow, and elsewhere. I think it will be a pretty common point of confusion as more folks start using .NET Core.

Perhaps should have started a broader dialog on this for 2.0. The usage is quite significant.
https://apisof.net/catalog/System.Diagnostics.ProcessStartInfo.UseShellExecute

I assume the OneCore issue can be solved by simply catching MissingMethodException. And I believe we support STA threads as you're not on Nano. (Where I guess ShellExecuteEx isn't available anyway)

Worth proposing as a DCR @stephentoub @mellinoe @Priya91 ?

Yes, we should try to do it for 2.0. It's fairly impactful.

@danmosemsft This will require flipping the default value of the property on desktop to false or providing implementation for true on Core, and making default on Core to true? I'm not sure what the process for DCR is, I'll be on vacation starting Monday, so someone else will have to drive it, to get it in before 2.0..

@Priya91 we discussed this offline (I think you were included) and believe the right thing is

  • on Windows, add support (throw for true where API is not there ie Windows Nano)
  • set default to false (better cloud optimized default as it's 10x faster also avoids divergent Nano behavior). Document this.
  • depending on feedback, consider quirking the ability to set default to true for an app (like desktop)
  • consider retrying with true if process start fails (would enable the URL scenario)

  • separately consider improving Unix implementation to copy Mono instead of just launching with sh.

Open question is whether to take this as a late DCR for 2.0.

Now please forget about this and enjoy your vacation!

This is done and ported into 2.0 馃憤

https://github.com/dotnet/corefx/issues/19956 covers Unix improvements.

Anything else, we can look for feedback.

I think the current non-implementation on Win32 is dangerous. First of all, UseShellExecute property acts as a no-op. Unlike UAP it doesn't even throw the PlatformNotSuppportedException. Secondly, I stumbled upon it when porting our desktop application because we use ProcessStartInfo to open URIs in a browser (for documentation and OAuth consent screens). That is currently not supported and breaks our application. Workarounds like running start or explorer to open URIs are problematic for long URIs, which get cut off.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nalywa picture nalywa  路  3Comments

jzabroski picture jzabroski  路  3Comments

bencz picture bencz  路  3Comments

v0l picture v0l  路  3Comments

yahorsi picture yahorsi  路  3Comments