[PATCH 0/2] MR3806: combase/roapi: Initialize implicit MTA in RoGetActivationFactory().
Fixes Starfield not being able to take photos in photo mode (due to failing to creating windowscodecs' image factory due to COM apartment being initialized). Turns out on Windows RoGetActivationFactory() initializes implicit MTA apartment when called from STA thread, and so after that called once COM is implicitly uninitialized for any (new) thread until COM is uninitialized in the thread which called RoGetActivationFactory (or that thread exited). This is not the case on Win8 (where the function was first introduced) but looks consistent after that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3806
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/combase/tests/roapi.c | 198 +++++++++++++++++++++++++++++++++++++ 1 file changed, 198 insertions(+) diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index f10cbb4507b..3959d7435b0 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -116,12 +116,210 @@ static void test_ActivationFactories(void) RoUninitialize(); } +static APTTYPE check_thread_apttype; +static APTTYPEQUALIFIER check_thread_aptqualifier; +static HRESULT check_thread_hr; + +static DWORD WINAPI check_apartment_thread(void *dummy) +{ + check_thread_apttype = 0xdeadbeef; + check_thread_aptqualifier = 0xdeadbeef; + check_thread_hr = CoGetApartmentType(&check_thread_apttype, &check_thread_aptqualifier); + return 0; +} + +#define check_thread_apartment(a) check_thread_apartment_(__LINE__, FALSE, a) +#define check_thread_apartment_broken(a) check_thread_apartment_(__LINE__, TRUE, a) +static void check_thread_apartment_(unsigned int line, BOOL broken_fail, HRESULT expected_hr_thread) +{ + HANDLE thread; + + check_thread_hr = 0xdeadbeef; + thread = CreateThread(NULL, 0, check_apartment_thread, NULL, 0, NULL); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + ok_(__FILE__, line)(check_thread_hr == expected_hr_thread + || broken(broken_fail && expected_hr_thread == S_OK && check_thread_hr == CO_E_NOTINITIALIZED), + "got %#lx, expected %#lx.\n", check_thread_hr, expected_hr_thread); + if (SUCCEEDED(check_thread_hr)) + { + ok_(__FILE__, line)(check_thread_apttype == APTTYPE_MTA, "got %d.\n", check_thread_apttype); + ok_(__FILE__, line)(check_thread_aptqualifier == APTTYPEQUALIFIER_IMPLICIT_MTA, "got %d.\n", check_thread_aptqualifier); + } +} + +static HANDLE mta_init_thread_init_done_event, mta_init_thread_done_event; + +static DWORD WINAPI mta_init_thread(void *dummy) +{ + HRESULT hr; + + hr = CoInitializeEx(NULL, COINIT_MULTITHREADED); + ok(hr == S_OK, "got %#lx.\n", hr); + SetEvent(mta_init_thread_init_done_event); + + WaitForSingleObject(mta_init_thread_done_event, INFINITE); + CoUninitialize(); + return 0; +} + +static DWORD WINAPI mta_init_implicit_thread(void *dummy) +{ + IActivationFactory *factory; + HSTRING str; + HRESULT hr; + + hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); + ok(hr == S_OK, "got %#lx.\n", hr); + + hr = WindowsCreateString(L"Does.Not.Exist", ARRAY_SIZE(L"Does.Not.Exist") - 1, &str); + ok(hr == S_OK, "got %#lx.\n", hr); + hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); + ok(hr == REGDB_E_CLASSNOTREG, "got %#lx.\n", hr); + WindowsDeleteString(str); + + SetEvent(mta_init_thread_init_done_event); + WaitForSingleObject(mta_init_thread_done_event, INFINITE); + + /* No CoUninitialize(), testing cleanup on thread exit. */ + return 0; +} + +static void test_implicit_mta(void) +{ + static const struct + { + BOOL ro_init; + BOOL mta; + BOOL ro_uninit; + } + tests[] = + { + { TRUE, TRUE, TRUE }, + { TRUE, FALSE, FALSE }, + { TRUE, FALSE, TRUE }, + { FALSE, FALSE, FALSE }, + { FALSE, FALSE, TRUE }, + }; + APTTYPEQUALIFIER aptqualifier; + IActivationFactory *factory; + APTTYPE apttype; + unsigned int i; + HANDLE thread; + HSTRING str; + HRESULT hr; + + hr = WindowsCreateString(L"Does.Not.Exist", ARRAY_SIZE(L"Does.Not.Exist") - 1, &str); + ok(hr == S_OK, "got %#lx.\n", hr); + /* RoGetActivationFactory doesn't implicitly initialize COM. */ + hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); + todo_wine ok(hr == CO_E_NOTINITIALIZED, "got %#lx.\n", hr); + + check_thread_apartment(CO_E_NOTINITIALIZED); + + /* RoGetActivationFactory initializes implicit MTA. */ + for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + winetest_push_context("test %u", i); + if (tests[i].ro_init) + hr = RoInitialize(tests[i].mta ? RO_INIT_MULTITHREADED : RO_INIT_SINGLETHREADED); + else + hr = CoInitializeEx(NULL, tests[i].mta ? COINIT_MULTITHREADED : COINIT_APARTMENTTHREADED); + ok(hr == S_OK, "got %#lx.\n", hr); + check_thread_apartment(tests[i].mta ? S_OK : CO_E_NOTINITIALIZED); + hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); + ok(hr == REGDB_E_CLASSNOTREG, "got %#lx.\n", hr); + todo_wine_if(!tests[i].mta) check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); + ok(hr == REGDB_E_CLASSNOTREG, "got %#lx.\n", hr); + todo_wine_if(!tests[i].mta) check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + if (tests[i].ro_uninit) + RoUninitialize(); + else + CoUninitialize(); + check_thread_apartment(CO_E_NOTINITIALIZED); + winetest_pop_context(); + } + + mta_init_thread_init_done_event = CreateEventW(NULL, FALSE, FALSE, NULL); + mta_init_thread_done_event = CreateEventW(NULL, FALSE, FALSE, NULL); + + /* RoGetActivationFactory references implicit MTA in a current thread + * even if implicit MTA was already initialized: check with STA init + * after RoGetActivationFactory(). */ + thread = CreateThread(NULL, 0, mta_init_thread, NULL, 0, NULL); + ok(!!thread, "failed.\n"); + WaitForSingleObject(mta_init_thread_init_done_event, INFINITE); + check_thread_apartment(S_OK); + + hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); + ok(hr == REGDB_E_CLASSNOTREG, "got %#lx.\n", hr); + check_thread_apartment(S_OK); + + hr = CoGetApartmentType(&apttype, &aptqualifier); + ok(hr == S_OK, "got %#lx.\n", hr); + ok(apttype == APTTYPE_MTA, "got %d.\n", apttype); + ok(aptqualifier == APTTYPEQUALIFIER_IMPLICIT_MTA, "got %d.\n", aptqualifier); + + hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); + ok(hr == S_OK, "got %#lx.\n", hr); + + hr = CoGetApartmentType(&apttype, &aptqualifier); + ok(hr == S_OK, "got %#lx.\n", hr); + ok(apttype == APTTYPE_MAINSTA, "got %d.\n", apttype); + ok(aptqualifier == APTTYPEQUALIFIER_NONE, "got %d.\n", aptqualifier); + + SetEvent(mta_init_thread_done_event); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + todo_wine check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + CoUninitialize(); + check_thread_apartment(CO_E_NOTINITIALIZED); + + /* RoGetActivationFactory references implicit MTA in a current thread + * even if implicit MTA was already initialized: check with STA init + * before RoGetActivationFactory(). */ + thread = CreateThread(NULL, 0, mta_init_thread, NULL, 0, NULL); + ok(!!thread, "failed.\n"); + WaitForSingleObject(mta_init_thread_init_done_event, INFINITE); + check_thread_apartment(S_OK); + + hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); + ok(hr == S_OK, "got %#lx.\n", hr); + + hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); + ok(hr == REGDB_E_CLASSNOTREG, "got %#lx.\n", hr); + check_thread_apartment(S_OK); + + SetEvent(mta_init_thread_done_event); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + todo_wine check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + CoUninitialize(); + check_thread_apartment(CO_E_NOTINITIALIZED); + + /* Test implicit MTA apartment thread exit. */ + thread = CreateThread(NULL, 0, mta_init_implicit_thread, NULL, 0, NULL); + ok(!!thread, "failed.\n"); + WaitForSingleObject(mta_init_thread_init_done_event, INFINITE); + todo_wine check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + SetEvent(mta_init_thread_done_event); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + check_thread_apartment(CO_E_NOTINITIALIZED); + + CloseHandle(mta_init_thread_init_done_event); + CloseHandle(mta_init_thread_done_event); + WindowsDeleteString(str); +} + START_TEST(roapi) { BOOL ret; load_resource(L"wine.combase.test.dll"); + test_implicit_mta(); test_ActivationFactories(); SetLastError(0xdeadbeef); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3806
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/combase/apartment.c | 32 ++++++++++++++++++++++++++++++++ dlls/combase/combase.c | 3 +++ dlls/combase/combase_private.h | 2 ++ dlls/combase/roapi.c | 5 +++++ dlls/combase/tests/roapi.c | 12 ++++++------ 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/dlls/combase/apartment.c b/dlls/combase/apartment.c index d679ef3407a..40d80114363 100644 --- a/dlls/combase/apartment.c +++ b/dlls/combase/apartment.c @@ -1157,6 +1157,11 @@ void leave_apartment(struct tlsdata *data) if (data->ole_inits) WARN( "Uninitializing apartment while Ole is still initialized\n" ); apartment_release(data->apt); + if (data->implicit_mta) + { + apartment_release(data->implicit_mta); + data->implicit_mta = NULL; + } data->apt = NULL; data->flags &= ~(OLETLS_DISABLE_OLE1DDE | OLETLS_APARTMENTTHREADED | OLETLS_MULTITHREADED); } @@ -1288,3 +1293,30 @@ void apartment_global_cleanup(void) apartment_release_dlls(); DeleteCriticalSection(&apt_cs); } + +HRESULT reference_implicit_mta_from_sta(void) +{ + struct tlsdata *data; + HRESULT hr; + struct apartment *apt, *apt_mt; + + if (FAILED(hr = com_get_tlsdata(&data))) + return hr; + if ((apt = data->apt) && (data->implicit_mta || apt->multi_threaded)) + return S_OK; + + EnterCriticalSection(&apt_cs); + if (apt && !mta) + apt_mt = mta = apartment_construct(COINIT_MULTITHREADED); + else if ((apt_mt = mta)) + apartment_addref(mta); + LeaveCriticalSection(&apt_cs); + + if (!apt_mt) + { + ERR("Apartment not initialized.\n"); + return CO_E_NOTINITIALIZED; + } + data->implicit_mta = apt_mt; + return S_OK; +} diff --git a/dlls/combase/combase.c b/dlls/combase/combase.c index f1b5828e0f8..08c83181c52 100644 --- a/dlls/combase/combase.c +++ b/dlls/combase/combase.c @@ -407,6 +407,9 @@ static void com_cleanup_tlsdata(void) if (tlsdata->apt) apartment_release(tlsdata->apt); + if (tlsdata->implicit_mta) + apartment_release(tlsdata->implicit_mta); + if (tlsdata->errorinfo) IErrorInfo_Release(tlsdata->errorinfo); if (tlsdata->state) diff --git a/dlls/combase/combase_private.h b/dlls/combase/combase_private.h index 53932e9a357..5461a67f816 100644 --- a/dlls/combase/combase_private.h +++ b/dlls/combase/combase_private.h @@ -74,6 +74,7 @@ enum tlsdata_flags struct tlsdata { struct apartment *apt; + struct apartment *implicit_mta; /* mta referenced by roapi from sta thread */ IErrorInfo *errorinfo; DWORD thread_seqid; /* returned with CoGetCurrentProcess */ DWORD flags; /* tlsdata_flags (+0Ch on x86) */ @@ -161,6 +162,7 @@ void apartment_release(struct apartment *apt); struct apartment * apartment_get_current_or_mta(void); HRESULT apartment_increment_mta_usage(CO_MTA_USAGE_COOKIE *cookie); void apartment_decrement_mta_usage(CO_MTA_USAGE_COOKIE cookie); +HRESULT reference_implicit_mta_from_sta(void); struct apartment * apartment_get_mta(void); HRESULT apartment_get_inproc_class_object(struct apartment *apt, const struct class_reg_data *regdata, REFCLSID rclsid, REFIID riid, DWORD class_context, void **ppv); diff --git a/dlls/combase/roapi.c b/dlls/combase/roapi.c index b80ca2e8325..bb7351db8a0 100644 --- a/dlls/combase/roapi.c +++ b/dlls/combase/roapi.c @@ -24,6 +24,8 @@ #include "roerrorapi.h" #include "winstring.h" +#include "combase_private.h" + #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(combase); @@ -154,6 +156,9 @@ HRESULT WINAPI RoGetActivationFactory(HSTRING classid, REFIID iid, void **class_ if (!iid || !class_factory) return E_INVALIDARG; + if (FAILED(hr = reference_implicit_mta_from_sta())) + return hr; + hr = get_library_for_classid(WindowsGetStringRawBuffer(classid, NULL), &library); if (FAILED(hr)) { diff --git a/dlls/combase/tests/roapi.c b/dlls/combase/tests/roapi.c index 3959d7435b0..ef0035dae4e 100644 --- a/dlls/combase/tests/roapi.c +++ b/dlls/combase/tests/roapi.c @@ -213,7 +213,7 @@ static void test_implicit_mta(void) ok(hr == S_OK, "got %#lx.\n", hr); /* RoGetActivationFactory doesn't implicitly initialize COM. */ hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); - todo_wine ok(hr == CO_E_NOTINITIALIZED, "got %#lx.\n", hr); + ok(hr == CO_E_NOTINITIALIZED, "got %#lx.\n", hr); check_thread_apartment(CO_E_NOTINITIALIZED); @@ -229,10 +229,10 @@ static void test_implicit_mta(void) check_thread_apartment(tests[i].mta ? S_OK : CO_E_NOTINITIALIZED); hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); ok(hr == REGDB_E_CLASSNOTREG, "got %#lx.\n", hr); - todo_wine_if(!tests[i].mta) check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + check_thread_apartment_broken(S_OK); /* Broken on Win8. */ hr = RoGetActivationFactory(str, &IID_IActivationFactory, (void **)&factory); ok(hr == REGDB_E_CLASSNOTREG, "got %#lx.\n", hr); - todo_wine_if(!tests[i].mta) check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + check_thread_apartment_broken(S_OK); /* Broken on Win8. */ if (tests[i].ro_uninit) RoUninitialize(); else @@ -272,7 +272,7 @@ static void test_implicit_mta(void) SetEvent(mta_init_thread_done_event); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); - todo_wine check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + check_thread_apartment_broken(S_OK); /* Broken on Win8. */ CoUninitialize(); check_thread_apartment(CO_E_NOTINITIALIZED); @@ -294,7 +294,7 @@ static void test_implicit_mta(void) SetEvent(mta_init_thread_done_event); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); - todo_wine check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + check_thread_apartment_broken(S_OK); /* Broken on Win8. */ CoUninitialize(); check_thread_apartment(CO_E_NOTINITIALIZED); @@ -302,7 +302,7 @@ static void test_implicit_mta(void) thread = CreateThread(NULL, 0, mta_init_implicit_thread, NULL, 0, NULL); ok(!!thread, "failed.\n"); WaitForSingleObject(mta_init_thread_init_done_event, INFINITE); - todo_wine check_thread_apartment_broken(S_OK); /* Broken on Win8. */ + check_thread_apartment_broken(S_OK); /* Broken on Win8. */ SetEvent(mta_init_thread_done_event); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3806
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=137319 Your paranoid android. === debian11b (64 bit WoW report) === ole32: compobj.c:4331: Test failed: Unexpected flags 0. compobj.c:4339: Test failed: Unexpected flags 0. compobj.c:4348: Test failed: Unexpected flags 0. compobj.c:4368: Test failed: Unexpected flags 0.
participants (3)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)