contextfree / winrt-rust

Use and (eventually) make Windows Runtime APIs with Rust
Apache License 2.0
142 stars 10 forks source link

RO_INIT_MULTITHREADED doesn’t work for everything #62

Closed chris-morgan closed 5 years ago

chris-morgan commented 5 years ago

I have not taken the time to grasp the difference between the different ROINITTHREADED values (I have done literally no research, and I won’t have a chance to until next week), but what I observe* is that this code:

use winrt::{RtActivatable};
use winrt::windows::web::ui::interop::{WebViewControlProcess, WebViewControlProcessOptions};

let options = WebViewControlProcessOptions::get_activation_factory()
    .activate_instance()
    .query_interface()
    .unwrap();
WebViewControlProcess::create_with_options(&options);

(Ugh, is that activation factory business really how to do it? It took me a while to get that far, but I think it’s about right, which is very icky! But that’s by the bye.)

… failed with code 0x8000001D, which is RO_E_UNSUPPORTED_FROM_MTA, “Activating a single-threaded class from MTA is not supported.”

As soon as I replaced RuntimeContext::init() with RoInitialize(RO_INIT_SINGLETHREADED), it started working fine.

I do not know how all of this works with reference to #60—as I said, I know nothing at present about the different apartment models beyond the names and very vague intuitions. @rbtying, are you perchance an expert?

I know not whether it should switch to single-threaded, or take an argument to choose.

rbtying commented 5 years ago

Unfortunately, I wouldn't call myself an expert in WinRT threading, and I'm not sure what's causing the issue you're seeing.

That said, it probably makes sense to expose the threading model setting for the library -- presumably the various settings exist for a reason.

On Thu, Jan 24, 2019, 5:38 AM Chris Morgan <notifications@github.com wrote:

I have not taken the time to grasp the difference between the different ROINITTHREADED values (I have done literally no research, and I won’t have a chance to until next week), but what I observe* is that this code:

use winrt::{RtActivatable};

use winrt::windows::web::ui::interop::{WebViewControlProcess, WebViewControlProcessOptions};

let options = WebViewControlProcessOptions::get_activation_factory()

.activate_instance()

.query_interface()

.unwrap();

WebViewControlProcess::create_with_options(&options);

(Ugh, is that activation factory business really how to do it? It took me a while to get that far, but I think it’s about right, which is very icky! But that’s by the bye.)

… failed with code 0x8000001D, which is RO_E_UNSUPPORTED_FROM_MTA, “Activating a single-threaded class from MTA is not supported.”

As soon as I replaced RuntimeContext::init() with RoInitialize(RO_INIT_SINGLETHREADED), it started working fine.

I do not know how all of this works with reference to #60 https://github.com/contextfree/winrt-rust/issues/60—as I said, I know nothing at present about the different apartment models beyond the names and very vague intuitions. @rbtying https://github.com/rbtying, are you perchance an expert?

I know not whether it should switch to single-threaded, or take an argument to choose.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/contextfree/winrt-rust/issues/62, or mute the thread https://github.com/notifications/unsubscribe-auth/AA4FwT9jicxenNj1jHyPFyVsXk12T-Wbks5vGbc5gaJpZM4aQ4qF .

BenJKuhn commented 5 years ago

I can try to help out a bit here.

WinRT borrows / shares a simplified apartment model with COM. There are two types of apartments, a multithreaded apartment (MTA), which is a singleton, and single-threaded apartments (STA). Single-threaded apartments are strongly associated 1:1 with a single thread. There can be any number of STAs. Any thread which hasn't explicitly opted into an apartment is assumed to be associated with the MTA. Apartments are the unit of dispatch of COM / WinRT marshalled calls (cross-thread or cross-process). A call into an object created from a thread bound to an STA is guaranteed to be delivered to a specific thread, which is often important for UI code correctness and performance. Marshalling and moving objects across threads is a more complex topic, but if you can ensure that the object is only used on the thread you created it on, you can avoid the more complex bits.

WebView is an example of a class that enforces that it be used from a thread associated with an STA. Most non-UI related WinRT objects can be used without issue from either an MTA or STA, so it's unlikely this will cause impact to other use of WinRT types within your code.

I'm not a rust expert by a long shot, but initializing an STA on the thread that's using the WebView is the correct fix for this issue.

Let me know if there's something you'd like me to clarify further.

Thanks,

Ben Kuhn (msft)

Boddlnagg commented 5 years ago

@chris-morgan I cannot say much about MTA/STA yet, because I need to read up on that first, but here's a quick comment about the activation factory business: You should be able to write

let options = WebViewControlProcessOptions::new();

which is provided by the RtDefaultConstructible trait. Now if you say that this needs to be better documented, I agree :-)

Boddlnagg commented 5 years ago

About MTA/STA: I think we need to have something like RuntimeContext::init_singlethreaded(), which under the hood uses RO_INIT_SINGLETHREADED. Furthermore, I'm not sure whether both init and init_singlethreaded should return Result<RuntimeContext>, because they could legitimately fail with RPC_E_CHANGED_MODE. Is this a programmer's error in all cases? Then an assert/panic should be fine. But if there could be something in the environment (which is outside the programmer's control) leading to that error, it should return Result. I think this could happen when the initialization is done in a library, and the main program has also already initialized the runtime (with the wrong/different init type).

rbtying commented 5 years ago

The vast majority of APIs work on both multithreaded and singlethreaded / apartmentthreaded. It's not clear to me that it is worthwhile to have RuntimeContext::as_singlethreaded_context() -> Result<&SingleThreadedRuntimeContext>, since the programmer is just going to expect that, and the runtime error should be handled anyways (as with all COM errors)

Boddlnagg commented 5 years ago

What happens when an application initializes with STA, and uses a library (I'm thinking about something like https://github.com/allenbenz/winrt-notification) which initializes with MTA (because that's the "default") ... that's going to be a problem, right? So the library either needs to know how the application initialized its WinRT context, or it should not do any initialization at all and expect everything to be initialized already ...

rbtying commented 5 years ago

Each thread initializes separately, and initialization can return S_FALSE which is "you're already initialized but not necessarily with the parameters you want". In general only the UI thread / main thread will use a non-multithreaded context, and that's the thread that the user most likely directly interacts with.

Changing the default here is probably a breaking change anyways?

Boddlnagg commented 5 years ago

I wouldn't worry too much about breaking changes ... the second sentence in this project's README is:

This library is still subject to breaking changes

BenJKuhn commented 5 years ago

In general, in Windows, we leave that to the application level. If a library needs a specific threading model, it does so only on threads that it owns.

There's an additional potential option: The MTA exists as long as it has been requested at least once. You can default to the implicit MTA by incrementing the MTA reference count without binding the current thread to the MTA (see https://docs.microsoft.com/en-us/windows/desktop/api/combaseapi/nf-combaseapi-coincrementmtausage). If an app wants to use STA instead, they should initialize STA before creating any winrt objects, but a thread that is implicitly MTA can still be initialized explicitly to STA.

This approach could limit the need to use explicit initialization to only those that want STA behavior.

Ben

Boddlnagg commented 5 years ago

@BenJKuhn Thank you for that valuable information! However, I don't quite understand this yet: You say that that approach could limit the need to use explicit initialization, but how would the implicit initialization work? If I understand you correctly, the MTA reference count would still explicitly need to be incremented (using CoIncrementMTAUsage), but when would that happen?

chris-morgan commented 5 years ago

Useful knowledge, thanks. Especially RtDefaultConstructible! That definitely makes the code nicer.

Documentation is a tricky thing with this kind of library in the context of rustdoc.

Now if anyone happened to want to help me, I wouldn’t mind: https://github.com/chris-morgan/webviewcontrol-rs/issues/1, the WebViewControlProcess.create_web_view_control_async async operation is never completing and I can’t figure out why. I’ll make no further mention in this issue here of the concrete case.

BenJKuhn commented 5 years ago

Sorry, I'm going to be a little less helpful there. If there's some notion of module initialization, that's where I'd put it. I don't know what options rust gives you in this context.

Ben

Boddlnagg commented 5 years ago

Now according to https://www.youtube.com/watch?v=X41j_gzSwOY#t=26m38s it's (usually) no longer necessary in cppwinrt to call init_apartment, which is the equivalent to RuntimeContext::init() in winrt-rust. So I wondered how that's possible and found that get_activation_factory now automatically calls CoIncrementMTAUsage when the call fails with an "uninitialized" error (https://github.com/microsoft/xlang/blob/e655fd0ad64298e576c4e19ecfa16c3f9745f9cd/src/tool/cppwinrt/strings/base_activation.h#L5-L19)

This is quite clever and I think we can do the same thing.

According to https://kennykerr.ca/2018/03/24/cppwinrt-hosting-the-windows-runtime/ it's also basically unnecessary to uninitialize the runtime, so we could get rid of the Drop impl that calls RoUninitialize.

(@rbtying What do you think about this, because you submitted #61, and the change there would now become obsolete?)

Additionally we have to add a multithreaded/singlethreaded parameter to RuntimeContext::init (we could even call it init_apartment because the struct is no longer required) so that STA initialization is possible (which was @chris-morgan's motivation for opening this thread).

conioh commented 1 year ago

I can try to help out a bit here.

WinRT borrows / shares a simplified apartment model with COM. There are two types of apartments, a multithreaded apartment (MTA), which is a singleton, and single-threaded apartments (STA). Single-threaded apartments are strongly associated 1:1 with a single thread. There can be any number of STAs. Any thread which hasn't explicitly opted into an apartment is assumed to be associated with the MTA. Apartments are the unit of dispatch of COM / WinRT marshalled calls (cross-thread or cross-process). A call into an object created from a thread bound to an STA is guaranteed to be delivered to a specific thread, which is often important for UI code correctness and performance. Marshalling and moving objects across threads is a more complex topic, but if you can ensure that the object is only used on the thread you created it on, you can avoid the more complex bits.

WebView is an example of a class that enforces that it be used from a thread associated with an STA. Most non-UI related WinRT objects can be used without issue from either an MTA or STA, so it's unlikely this will cause impact to other use of WinRT types within your code.

I'm not a rust expert by a long shot, but initializing an STA on the thread that's using the WebView is the correct fix for this issue.

Let me know if there's something you'd like me to clarify further.

Thanks,

Ben Kuhn (msft)

@BenJKuhn, a question that arises here and I couldn't find documentation about is this:

In COM, that WinRT "borrows" from, a caller in any apartment can call an object if any threading model, given that there's a marshaller registered for it. (I'm ignoring "Main STA" and NT/NTA in the following.)

Suddenly in WinRT the first 3 items are still true but the 4th one isn't. In COM, if the object is compatible with the instantiator's apartment it will be instantiated on it. If it is incompatible, the object will be instantiated on an appropriate apartment (the MTA or a new STA) and a proxy will be returned to the instantiator. But in WinRT the case of incompatible apartment works only in one direction. an STA can create an MTA object but an MTA can't create an STA object. Instead it gets RO_E_UNSUPPORTED_FROM_MTA. As is evident by the RO prefix this is new behavior in WinRT. There's no generic COM E_UNSUPPORTED_FROM_MTA , only RO_....

Could you provide some information on this and rationale for this behavior?