dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

(Optionally) make Thread.SetApartmentState behavior to match [STAThread] on Main() on non-Windows platforms. #100639

Open kekekeks opened 5 months ago

kekekeks commented 5 months ago

Existing (3rd party) library code is using Thread.SetApartmentState to make its worker threads compatible with various Win32 APIs.

On non-windows platforms, this method throws a PlatformNotSupportedException, which makes an otherwise compatible library code (that by itself doesn't even use Win32 APIs) impossible to consume on such platforms.

At the same time STAThreadAttribute applied on Main does nothing and doesn't prevent the app from running on those platforms, so there is already a case when .NET completely ignores desired apartment and it's considered a completely safe thing to do.

The proposal is to have a .runtimeconfig.json flag, that would prevent Thread.SetApartmentState from throwing exceptions on non-Windows platforms.

jkotas commented 5 months ago

Can the library be fixed to call Thread.SetApartmentState only on Windows?

The proposal is to have a .runtimeconfig.json flag, that would prevent Thread.SetApartmentState from throwing exceptions on non-Windows platforms.

We add compatibility quirks like this as the very last resort for issues with broad impact. I do not think this compatibility quirk meets the bar.

Zintom commented 5 months ago

Can the library be fixed to call Thread.SetApartmentState only on Windows?

Yeah a #IF would suffice in the lib code

kekekeks commented 5 months ago

The library code is a 3rd party one and we can't IL-patch it due to licensing restrictions, unfortunately.

kekekeks commented 5 months ago

Why does the method throw in the first place, BTW? Since [STAThread] is silently ignored instead of throwing, the runtime already considers some apartment-related APIs to be a no-op on non-windows.

jkotas commented 5 months ago

In general, the API behaviors on unsupported platforms are:

(I am sure you would be able to find some counter-examples.)

AaronRobinsonMSFT commented 5 months ago

The library code is a 3rd party one and we can't IL-patch it due to licensing restrictions, unfortunately.

Something to consider is it seems the 3rd party doesn't support running on non-Windows platforms. It is quite possible additional issues are present and this is simply the first of many. It also means it isn't tested on those platforms so relying on it is exposing applications to added risk.

It would seem most appropriate to reach out to the library author and work with them on a support non-Windows option.

kekekeks commented 5 months ago

We are using NativeLibrary.SetDllImportResolver and a native lib implementing win32 apis to fix the rest of "incompatibilities". The rest of the code is compatible.

jkotas commented 5 months ago

What are you going to do if an updated version of the library introduces more Windows-specific code that is not possible to "fix" using NativeLibrary.SetDllImportResolver?

The support plan you are on does not look dependable.

kekekeks commented 5 months ago

What are you going to do if an updated version of the library introduces more Windows-specific code that is not possible to "fix" using NativeLibrary.SetDllImportResolver?

We've managed to get lots of 3rd party WPF components to work flawlessly using that API.

So far we've only had 2 roadblocks that can't be fixed on our side and both are COM-related and are explicitly blocked by runtime and not by 3rd party code doing something windows-specific.

1) HostVisual/VisualTarget require an STA thread with windows-only version of WPF and don't require it with xplat-one, but pre-existing 3rd party libs (some of those haven't seen updates for half a decade) are setting the STA thread flag just to get those 100% managed APIs to work.

2) Another COM-related major blocker is people trying to use TF_CreateThreadMgr which the runtime simply refuses to call even though our implementation returns E_FAIL and don't provide anything in ppv, but that's a separate story. For that one, I believe, we'll have to build and ship a custom runtime