Add check if STA is supported before using on console startup #15106
Conversation
src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.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. |
|
I've opened an issue to ask the .NET team about this, however as I wrote it some thoughts occurred to me:
|
|
The problem is that we default to STA for compatibility with Windows PowerShell which is needed for winforms/WPF which requires STA. This was a breaking change from PowerShell Core 6 (which defaulted to MTA) with PowerShell 7 and is affecting some users who don't even care about winforms/WPF. Whether STA or MTA is used is needed to be determined before we create the first runspace. The exception occurs when the console host tries to open the runspace as that is when the thread is actually created. I don't think it makes sense for the API to create the runspace to override STA and magically make it MTA. Doing it in the host makes more sense to me as the user isn't explicitly requesting it (unless |
I wonder - no exception is on the SKUs from SetApartmentState()? I see we use StaMode property only in one place. And I guess the issue is in and I believe we could not add a test in StaMode but add a fix in the two places. It would be more reliable solution. |
|
The exception doesn't occur with calling |
My thought just to start a thread and process the exception. Cases (I don't know that is right):
|
|
Got an answer from COM team on a better way to check if STA is supported but requires a PInvoke. |
There was a problem hiding this comment.
Based on the suggestions in the .NET issue, it might be more efficient and threadsafe if this is done lazily:
public static bool IsStaSupported
{
get
{
#if UNIX
return false;
#else
return s_isStaSupportedLazy.Value;
#endif
}
}
private static readonly Lazy<bool> s_isStaSupported = new Lazy<bool>(GetStaSupportedValue);
private static bool GetStaSupportedValue()
{
// See objbase.h
const int COINIT_APARTMENTTHREADED = 0x2;
const int E_NOTIMPL = unchecked((int)0X80004001);
int result = Windows.NativeMethods.CoInitializeEx(IntPtr.Zero, COINIT_APARTMENTTHREADED);
// If 0 is returned the thread has been initialized for the first time
// as an STA and therefore must remain that way.
if (result > 0)
{
Windows.NativeMethods.CoUninitialize();
}
return result != E_NOTIMPL;
}
I suspect that despite being able to make our own migration (although that would require more work that currently isn't planned), users with existing scripts that use WPF expect continued compatibility. |
I think it would be useful for us to summarise the state of things so far in terms of which PowerShell version has what behaviour and what are the motivating scenarios. Is the following accurate?
Some questions:
|
Yes.
No.
We just followed default .Net behavior that is MTA on all platforms. It was ok until .Net ported WPF to .Net Core and we retrieved WPF to pwsh too.
I am not sure I understand the phrase. While pwsh was MTA the COM subsystem did set STA explicitly so COM support was still working. And this keeps on working the same today.
Good question :-)
Interesting, PowerShell 2 default is MTA. But it raised issues with GUI scripts and default was changed to STA in PowerShell 3.
If we set MTA and call an GUI control we get an exception.
|
|
Regarding PSCore6, it was changed to MTA because PowerShell Core 5 (what was shipped on NanoServer originally) was MTA and this is because NanoServer doesn't support STA. I'm pretty sure .NET Core didn't support STA initially until later when it was at "parity" with .NET Framework. For PowerShell 7, we did change the default back to STA because of support for WPF/WinForms that was added to .NET Core. Once STA or MTA is set on a thread, you can't change it. So once the default runspace created by ConsoleHost (in this case) has set the ApartmentState, that's it. This is why the switches exist for pwsh.exe and allow to be set if the user knows they are calling some COM API that requires one or the other to work successfully. I think the only scenario PowerShell users have encountered STA vs MTA is using winforms/WPF from script. The cases where Windows itself doesn't support STA, the usage of PowerShell is as a console or automation and I haven't heard of any scenarios where they needed STA (or aware of the problem until they try PS7). |
|
So for example, a set of behaviours that would make sense to me is:
I don't love that the default isn't simply the implicit version of an explicit configuration, and I've seen plenty of docs where the default or undefined configuration seems innocent enough but ends up being very complex because it tortuously tries to "just work". In this circumstance though, I don't think the default is that complicated -- you either end up in STA, or if that's not supported, MTA. The important part for me is that when you explicitly set an option, you either get the configured result or it crashes, there's no "suggestion noted" behaviour where we quietly just give you MTA when you use It does raise the question to me of providing the |
It is ideal.
In history the default was changed thrice! So it is better to keep the parameters :-) |
| // See objbase.h | ||
| const int COINIT_APARTMENTTHREADED = 0x2; | ||
| const int E_NOTIMPL = unchecked((int)0X80004001); | ||
| int result = Windows.NativeMethods.CoInitializeEx(IntPtr.Zero, COINIT_APARTMENTTHREADED); |
There was a problem hiding this comment.
I wonder if this PInvoke is anymore efficient than spinning up a thread before. Has any perf testing been done?
| // See objbase.h | ||
| const int COINIT_APARTMENTTHREADED = 0x2; | ||
| const int E_NOTIMPL = unchecked((int)0X80004001); | ||
| int result = Windows.NativeMethods.CoInitializeEx(IntPtr.Zero, COINIT_APARTMENTTHREADED); |
There was a problem hiding this comment.
Can an exception be thrown here?
There was a problem hiding this comment.
CoInitializeEx only returns an HRESULT, so there should never be an exception here unless .NET throws.
|
Leaving a question here in anticipation of the @PowerShell/powershell-committee on this. I'm fairly ignorant of apartment state in general, and my knowledge is limited to the kind of historical context that @SteveL-MSFT referenced above. In what sort of situations would a user care about their apartment state beyond the two scenarios relevant in this issue (i.e. whether or not WPF/WinForms is supported, and whether or not pwsh starts up in the default case on some Windows SKUs). @rjmholt, you alluded to this a little bit in some of your responses around the user needing to hold a flow chart in their head around apartment state. But what sort of user needs to do that, and for what? |
|
@PowerShell/powershell-committee reviewed this and is ok with handling the check in the consolehost so that it starts on Windows skus that don't support STA and explicitly specifying |
| // If 0 is returned the thread has been initialized for the first time | ||
| // as an STA and thus supported and needs to be uninitialized. | ||
| if (result > 0) | ||
| { | ||
| Windows.NativeMethods.CoUninitialize(); | ||
| } |
There was a problem hiding this comment.
I wonder why we need the check if it is used in startup scenario.
If we want to have universal code the condition looks wrong:
S_Ok = 0
S_False = 1
RPC_E_CHANGED_MODE = -2147417850
There was a problem hiding this comment.
It's needed in the startup scenario as the main console runspace needs to start as either STA or MTA. So we need to know if STA is supported before trying STA otherwise pwsh ends up in an unusable state. S_False will never apply here as CoInitialize() won't be called before this.
|
Isn't it easier to just check this System.Threading.Thread.CurrentThread.ApartmentState? By default we start in STA on Windows but if an Windows build doesn't support STA it start in MTA mode. Yes? |
|
No, by default ApartmentState is |
|
Interes 10000 ting, in debugger I see MTA for main thread. This brings us back to the idea that try-fallback (and cache result) approach should work even more reliable and predictable. |
|
The @PowerShell/powershell-committee discussed the option to have runspace creation handle the problem of STA support, but decided against it as ApartmentState is set via the Runspace API and should be respected. The console, however, is not something most users need to think about STA vs MTA and should just start and for advanced users, they can rely on the |
If it is all about "You only need STA or MTA to interop with COM." the proposal with fallback in (Such fallback is used already in New-Object but in another scenario.) |
|
🎉 Handy links: |
PR Summary
Some specific builds of Windows do not support STA. However, pwsh defaults to STA to support WinForms/WPF (same as Windows PowerShell). This results in an error and unusable PowerShell runspace. Fix is to have a check whether
-STAis explicitly used or implicitly where-MTAis not specified by attempting to create a STA thread.Validated manually.
PR Context
Reported by internal Msft partner
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.(which runs in a different PS Host).