Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/ole32/tests/compobj.c | 228 +++++++++------------------------------------ 1 file changed, 46 insertions(+), 182 deletions(-)
diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c index f1e0134..0ce941b 100644 --- a/dlls/ole32/tests/compobj.c +++ b/dlls/ole32/tests/compobj.c @@ -602,25 +602,6 @@ static void test_StringFromGUID2(void) ok(len == 0, "len: %d (expected 0)\n", len); }
-struct info -{ - HANDLE wait, stop; -}; - -static DWORD CALLBACK ole_initialize_thread(LPVOID pv) -{ - HRESULT hr; - struct info *info = pv; - - hr = pCoInitializeEx(NULL, COINIT_MULTITHREADED); - - SetEvent(info->wait); - WaitForSingleObject(info->stop, 10000); - - CoUninitialize(); - return hr; -} - #define test_apt_type(t, q, t_t, t_q) _test_apt_type(t, q, t_t, t_q, __LINE__) static void _test_apt_type(APTTYPE expected_type, APTTYPEQUALIFIER expected_qualifier, BOOL todo_type, BOOL todo_qualifier, int line) @@ -644,10 +625,7 @@ todo_wine_if(todo_qualifier) static void test_CoCreateInstance(void) { HRESULT hr; - HANDLE thread; - DWORD tid, exitcode; IUnknown *pUnk; - struct info info; REFCLSID rclsid = &CLSID_InternetZoneManager;
pUnk = (IUnknown *)0xdeadbeef; @@ -682,51 +660,15 @@ static void test_CoCreateInstance(void) hr = CoCreateInstance(rclsid, NULL, CLSCTX_INPROC_SERVER, &IID_IUnknown, (void **)&pUnk); ok(hr == CO_E_NOTINITIALIZED, "CoCreateInstance should have returned CO_E_NOTINITIALIZED instead of 0x%08x\n", hr);
- /* show that COM doesn't have to be initialized for multi-threaded apartments if another - thread has already done so */ - - test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE); - - info.wait = CreateEventA(NULL, TRUE, FALSE, NULL); - ok(info.wait != NULL, "CreateEvent failed with error %d\n", GetLastError()); - - info.stop = CreateEventA(NULL, TRUE, FALSE, NULL); - ok(info.stop != NULL, "CreateEvent failed with error %d\n", GetLastError()); - - thread = CreateThread(NULL, 0, ole_initialize_thread, &info, 0, &tid); - ok(thread != NULL, "CreateThread failed with error %d\n", GetLastError()); - - ok( !WaitForSingleObject(info.wait, 10000 ), "wait timed out\n" ); - - test_apt_type(APTTYPE_MTA, APTTYPEQUALIFIER_IMPLICIT_MTA, TRUE, TRUE); - - pUnk = (IUnknown *)0xdeadbeef; - hr = CoCreateInstance(rclsid, NULL, CLSCTX_INPROC_SERVER, &IID_IUnknown, (void **)&pUnk); - ok(hr == S_OK, "CoCreateInstance should have returned S_OK instead of 0x%08x\n", hr); - if (pUnk) IUnknown_Release(pUnk); - - SetEvent(info.stop); - ok( !WaitForSingleObject(thread, 10000), "wait timed out\n" ); - - GetExitCodeThread(thread, &exitcode); - hr = exitcode; - ok(hr == S_OK, "thread should have returned S_OK instead of 0x%08x\n", hr); - - CloseHandle(thread); - CloseHandle(info.wait); - CloseHandle(info.stop); - test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE); }
static void test_CoGetClassObject(void) { HRESULT hr; - HANDLE thread, handle; - DWORD tid, exitcode; + HANDLE handle; ULONG_PTR cookie; IUnknown *pUnk; - struct info info; REFCLSID rclsid = &CLSID_InternetZoneManager; HKEY hkey; LONG res; @@ -740,45 +682,6 @@ static void test_CoGetClassObject(void) broken(hr == CO_E_NOTINITIALIZED), /* win9x */ "CoGetClassObject should have returned E_INVALIDARG instead of 0x%08x\n", hr);
- /* show that COM doesn't have to be initialized for multi-threaded apartments if another - thread has already done so */ - - test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE); - - info.wait = CreateEventA(NULL, TRUE, FALSE, NULL); - ok(info.wait != NULL, "CreateEvent failed with error %d\n", GetLastError()); - - info.stop = CreateEventA(NULL, TRUE, FALSE, NULL); - ok(info.stop != NULL, "CreateEvent failed with error %d\n", GetLastError()); - - thread = CreateThread(NULL, 0, ole_initialize_thread, &info, 0, &tid); - ok(thread != NULL, "CreateThread failed with error %d\n", GetLastError()); - - ok( !WaitForSingleObject(info.wait, 10000), "wait timed out\n" ); - - test_apt_type(APTTYPE_MTA, APTTYPEQUALIFIER_IMPLICIT_MTA, TRUE, TRUE); - - pUnk = (IUnknown *)0xdeadbeef; - hr = CoGetClassObject(rclsid, CLSCTX_INPROC_SERVER, NULL, &IID_IUnknown, (void **)&pUnk); - if(hr == REGDB_E_CLASSNOTREG) - skip("IE not installed so can't test CoGetClassObject\n"); - else - { - ok(hr == S_OK, "CoGetClassObject should have returned S_OK instead of 0x%08x\n", hr); - if (pUnk) IUnknown_Release(pUnk); - } - - SetEvent(info.stop); - ok( !WaitForSingleObject(thread, 10000), "wait timed out\n" ); - - GetExitCodeThread(thread, &exitcode); - hr = exitcode; - ok(hr == S_OK, "thread should have returned S_OK instead of 0x%08x\n", hr); - - CloseHandle(thread); - CloseHandle(info.wait); - CloseHandle(info.stop); - test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE);
if (!pRegOverridePredefKey) @@ -1880,9 +1783,6 @@ static void test_CoGetObjectContext(void) IObjContext *pObjContext; APTTYPE apttype; THDTYPE thdtype; - struct info info; - HANDLE thread; - DWORD tid, exitcode; GUID id, id2;
if (!pCoGetObjectContext) @@ -1895,27 +1795,12 @@ static void test_CoGetObjectContext(void) ok(hr == CO_E_NOTINITIALIZED, "CoGetObjectContext should have returned CO_E_NOTINITIALIZED instead of 0x%08x\n", hr); ok(pComThreadingInfo == NULL, "pComThreadingInfo should have been set to NULL\n");
- /* show that COM doesn't have to be initialized for multi-threaded apartments if another - thread has already done so */ - - test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE); - - info.wait = CreateEventA(NULL, TRUE, FALSE, NULL); - ok(info.wait != NULL, "CreateEvent failed with error %d\n", GetLastError()); - - info.stop = CreateEventA(NULL, TRUE, FALSE, NULL); - ok(info.stop != NULL, "CreateEvent failed with error %d\n", GetLastError()); - - thread = CreateThread(NULL, 0, ole_initialize_thread, &info, 0, &tid); - ok(thread != NULL, "CreateThread failed with error %d\n", GetLastError()); - - ok( !WaitForSingleObject(info.wait, 10000), "wait timed out\n" ); + pCoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
- test_apt_type(APTTYPE_MTA, APTTYPEQUALIFIER_IMPLICIT_MTA, TRUE, TRUE); + test_apt_type(APTTYPE_MAINSTA, APTTYPEQUALIFIER_NONE, FALSE, FALSE);
- pComThreadingInfo = NULL; hr = pCoGetObjectContext(&IID_IComThreadingInfo, (void **)&pComThreadingInfo); - ok(hr == S_OK, "Expected S_OK, got 0x%08x\n", hr); + ok_ole_success(hr, "CoGetObjectContext");
threadinginfo2 = NULL; hr = pCoGetObjectContext(&IID_IComThreadingInfo, (void **)&threadinginfo2); @@ -1933,28 +1818,6 @@ static void test_CoGetObjectContext(void) hr = CoGetCurrentLogicalThreadId(&id2); ok(IsEqualGUID(&id, &id2), "got %s, expected %s\n", wine_dbgstr_guid(&id), wine_dbgstr_guid(&id2));
- IComThreadingInfo_Release(pComThreadingInfo); - - SetEvent(info.stop); - ok( !WaitForSingleObject(thread, 10000), "wait timed out\n" ); - - GetExitCodeThread(thread, &exitcode); - hr = exitcode; - ok(hr == S_OK, "thread should have returned S_OK instead of 0x%08x\n", hr); - - CloseHandle(thread); - CloseHandle(info.wait); - CloseHandle(info.stop); - - test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE); - - pCoInitializeEx(NULL, COINIT_APARTMENTTHREADED); - - test_apt_type(APTTYPE_MAINSTA, APTTYPEQUALIFIER_NONE, FALSE, FALSE); - - hr = pCoGetObjectContext(&IID_IComThreadingInfo, (void **)&pComThreadingInfo); - ok_ole_success(hr, "CoGetObjectContext"); - hr = IComThreadingInfo_GetCurrentApartmentType(pComThreadingInfo, &apttype); ok_ole_success(hr, "IComThreadingInfo_GetCurrentApartmentType"); ok(apttype == APTTYPE_MAINSTA, "apartment type should be APTTYPE_MAINSTA instead of %d\n", apttype); @@ -2118,9 +1981,6 @@ static void test_CoGetContextToken(void) ULONG refs; ULONG_PTR token, token2; IObjContext *ctx; - struct info info; - HANDLE thread; - DWORD tid, exitcode;
if (!pCoGetContextToken) { @@ -2133,44 +1993,6 @@ static void test_CoGetContextToken(void) ok(hr == CO_E_NOTINITIALIZED, "Expected CO_E_NOTINITIALIZED, got 0x%08x\n", hr); ok(token == 0xdeadbeef, "Expected 0, got 0x%lx\n", token);
- /* show that COM doesn't have to be initialized for multi-threaded apartments if another - thread has already done so */ - - test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE); - - info.wait = CreateEventA(NULL, TRUE, FALSE, NULL); - ok(info.wait != NULL, "CreateEvent failed with error %d\n", GetLastError()); - - info.stop = CreateEventA(NULL, TRUE, FALSE, NULL); - ok(info.stop != NULL, "CreateEvent failed with error %d\n", GetLastError()); - - thread = CreateThread(NULL, 0, ole_initialize_thread, &info, 0, &tid); - ok(thread != NULL, "CreateThread failed with error %d\n", GetLastError()); - - ok( !WaitForSingleObject(info.wait, 10000), "wait timed out\n" ); - - test_apt_type(APTTYPE_MTA, APTTYPEQUALIFIER_IMPLICIT_MTA, TRUE, TRUE); - - token = 0; - hr = pCoGetContextToken(&token); - ok(hr == S_OK, "Expected S_OK, got 0x%08x\n", hr); - - token2 = 0; - hr = pCoGetContextToken(&token2); - ok(hr == S_OK, "Expected S_OK, got 0x%08x\n", hr); - ok(token == token2, "got different token\n"); - - SetEvent(info.stop); - ok( !WaitForSingleObject(thread, 10000), "wait timed out\n" ); - - GetExitCodeThread(thread, &exitcode); - hr = exitcode; - ok(hr == S_OK, "thread should have returned S_OK instead of 0x%08x\n", hr); - - CloseHandle(thread); - CloseHandle(info.wait); - CloseHandle(info.stop); - test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE);
CoInitialize(NULL); @@ -3874,6 +3696,47 @@ static void init_funcs(void) pReleaseActCtx = (void*)GetProcAddress(hkernel32, "ReleaseActCtx"); }
+static DWORD CALLBACK implicit_mta_proc(void *param) +{ + IComThreadingInfo *threading_info; + ULONG_PTR token; + IUnknown *unk; + HRESULT hr; + + test_apt_type(APTTYPE_MTA, APTTYPEQUALIFIER_IMPLICIT_MTA, TRUE, TRUE); + + hr = CoCreateInstance(&CLSID_InternetZoneManager, NULL, CLSCTX_INPROC_SERVER, &IID_IUnknown, (void **)&unk); + ok_ole_success(hr, "CoCreateInstance"); + IUnknown_Release(unk); + + hr = CoGetClassObject(&CLSID_InternetZoneManager, CLSCTX_INPROC_SERVER, NULL, &IID_IUnknown, (void **)&unk); + ok_ole_success(hr, "CoGetClassObject"); + IUnknown_Release(unk); + + hr = CoGetObjectContext(&IID_IComThreadingInfo, (void **)&threading_info); + ok_ole_success(hr, "CoGetObjectContext"); + IComThreadingInfo_Release(threading_info); + + hr = CoGetContextToken(&token); + ok_ole_success(hr, "CoGetContextToken"); + + return 0; +} + +/* Some COM functions (perhaps even all of them?) can make use of an "implicit" + * multi-threaded apartment created by another thread in the same process. */ +static void test_implicit_mta(void) +{ + HANDLE thread; + + CoInitializeEx(NULL, COINIT_MULTITHREADED); + + thread = CreateThread(NULL, 0, implicit_mta_proc, NULL, 0, NULL); + ok(!WaitForSingleObject(thread, 1000), "wait failed\n"); + + CoUninitialize(); +} + START_TEST(compobj) { init_funcs(); @@ -3923,4 +3786,5 @@ START_TEST(compobj) test_IInitializeSpy(); test_CoGetInstanceFromFile(); test_GlobalOptions(); + test_implicit_mta(); }
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/ole32/compobj.c | 48 ++++++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 28 deletions(-)
diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c index cd0e67f..a317bf7 100644 --- a/dlls/ole32/compobj.c +++ b/dlls/ole32/compobj.c @@ -717,6 +717,23 @@ static inline BOOL apartment_is_model(const APARTMENT *apt, DWORD model) return (apt->multi_threaded == !(model & COINIT_APARTMENTTHREADED)); }
+/* gets the multi-threaded apartment if it exists. The caller must + * release the reference from the apartment as soon as the apartment pointer + * is no longer required. */ +static APARTMENT *apartment_find_mta(void) +{ + APARTMENT *apt; + + EnterCriticalSection(&csApartment); + + if ((apt = MTA)) + apartment_addref(apt); + + LeaveCriticalSection(&csApartment); + + return apt; +} + static void COM_RevokeRegisteredClassObject(RegisteredClass *curClass) { list_remove(&curClass->entry); @@ -1295,31 +1312,6 @@ static APARTMENT *apartment_findmain(void) return result; }
-/* gets the multi-threaded apartment if it exists. The caller must - * release the reference from the apartment as soon as the apartment pointer - * is no longer required. */ -static APARTMENT *apartment_find_multi_threaded(void) -{ - APARTMENT *result = NULL; - struct list *cursor; - - EnterCriticalSection(&csApartment); - - LIST_FOR_EACH( cursor, &apts ) - { - struct apartment *apt = LIST_ENTRY( cursor, struct apartment, entry ); - if (apt->multi_threaded) - { - result = apt; - apartment_addref(result); - break; - } - } - - LeaveCriticalSection(&csApartment); - return result; -} - /* gets the specified class object by loading the appropriate DLL, if * necessary and calls the DllGetClassObject function for the DLL */ static HRESULT apartment_getclassobject(struct apartment *apt, LPCWSTR dllpath, @@ -3000,7 +2992,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
if (!(apt = COM_CurrentApt())) { - if (!(apt = apartment_find_multi_threaded())) + if (!(apt = apartment_find_mta())) { ERR("apartment not initialised\n"); return CO_E_NOTINITIALIZED; @@ -3300,7 +3292,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoCreateInstanceEx(
if (!(apt = COM_CurrentApt())) { - if (!(apt = apartment_find_multi_threaded())) + if (!(apt = apartment_find_mta())) { ERR("apartment not initialised\n"); return CO_E_NOTINITIALIZED; @@ -5007,7 +4999,7 @@ HRESULT WINAPI CoGetContextToken( ULONG_PTR *token ) if (!info->apt) { APARTMENT *apt; - if (!(apt = apartment_find_multi_threaded())) + if (!(apt = apartment_find_mta())) { ERR("apartment not initialised\n"); return CO_E_NOTINITIALIZED;
Signed-off-by: Huw Davies huw@codeweavers.com
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- In a sense this is just to make the code simpler, but the problem it causes would happen anyway. The basic problem is that destroying an apartment cleans up its proxy manager, which calls proxy_manager_get_remunknown() in order to release the IRemUnknown if it exists, and this may end up unmarshalling the IRemUnknown, triggering calls to CoGetClassObject (as well as CoGetPSClsid() etc.) Later it's also necessary to call apartment_get_current_or_mta() in proxy_manager_get_remunknown() itself. As far as I understand we wouldn't tear down the proxy manager from a different thread than the MTA was initialized on, but we might call proxy_manager_get_remunknown() from a different thread, so this is necessary.
Alternate solutions, I guess: * Use the apartment's critical section instead. This seems existentially not quite appropriate, and it's not clear to me what all of the implications of this would be. * Try to restructure where the apartment is grabbed. * Perhaps don't even grab a reference to the MTA at all. This seems terrible on its face, but it also seems true that an app which closes the MTA while it's using an MTA object is pretty broken. There is quite a lot of DCOM about, so I'd at least appreciate help in determining whether this solution or any of the alternates is best.
dlls/ole32/compobj.c | 26 +++++++++++++++++--------- dlls/ole32/compobj_private.h | 1 + 2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c index a317bf7..1c7645b 100644 --- a/dlls/ole32/compobj.c +++ b/dlls/ole32/compobj.c @@ -1162,9 +1162,17 @@ DWORD apartment_release(struct apartment *apt)
ret = InterlockedDecrement(&apt->refs); TRACE("%s: after = %d\n", wine_dbgstr_longlong(apt->oxid), ret); + + if (apt->being_destroyed) + { + LeaveCriticalSection(&csApartment); + return ret; + } + /* destruction stuff that needs to happen under csApartment CS */ if (ret == 0) { + apt->being_destroyed = TRUE; if (apt == MTA) MTA = NULL; else if (apt == MainApartment) MainApartment = NULL; list_remove(&apt->entry); @@ -2981,7 +2989,6 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject( IUnknown *regClassObject; HRESULT hres = E_UNEXPECTED; APARTMENT *apt; - BOOL release_apt = FALSE;
TRACE("CLSID: %s,IID: %s\n", debugstr_guid(rclsid), debugstr_guid(iid));
@@ -2990,14 +2997,15 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
*ppv = NULL;
- if (!(apt = COM_CurrentApt())) + if ((apt = COM_CurrentApt())) + apartment_addref(apt); + else { if (!(apt = apartment_find_mta())) { ERR("apartment not initialised\n"); return CO_E_NOTINITIALIZED; } - release_apt = TRUE; }
if (pServerInfo) { @@ -3009,7 +3017,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject( { if (IsEqualCLSID(rclsid, &CLSID_InProcFreeMarshaler)) { - if (release_apt) apartment_release(apt); + apartment_release(apt); return FTMarshalCF_Create(iid, ppv); } if (IsEqualCLSID(rclsid, &CLSID_GlobalOptions)) @@ -3035,7 +3043,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
hres = get_inproc_class_object(apt, &clsreg, &comclass->clsid, iid, !(dwClsContext & WINE_CLSCTX_DONT_HOST), ppv); ReleaseActCtx(data.hActCtx); - if (release_apt) apartment_release(apt); + apartment_release(apt); return hres; } } @@ -3056,7 +3064,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject( * is good since we are not returning it in the "out" parameter. */ IUnknown_Release(regClassObject); - if (release_apt) apartment_release(apt); + apartment_release(apt); return hres; }
@@ -3091,7 +3099,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject( * other types */ if (SUCCEEDED(hres)) { - if (release_apt) apartment_release(apt); + apartment_release(apt); return hres; } } @@ -3127,11 +3135,11 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject( * other types */ if (SUCCEEDED(hres)) { - if (release_apt) apartment_release(apt); + apartment_release(apt); return hres; } } - if (release_apt) apartment_release(apt); + apartment_release(apt);
/* Next try out of process */ if (CLSCTX_LOCAL_SERVER & dwClsContext) diff --git a/dlls/ole32/compobj_private.h b/dlls/ole32/compobj_private.h index 9e65c3e..12413f7 100644 --- a/dlls/ole32/compobj_private.h +++ b/dlls/ole32/compobj_private.h @@ -142,6 +142,7 @@ struct apartment DWORD host_apt_tid; /* thread ID of apartment hosting objects of differing threading model (CS cs) */ HWND host_apt_hwnd; /* handle to apartment window of host apartment (CS cs) */ LocalServer *local_server; /* A marshallable object exposing local servers (CS cs) */ + BOOL being_destroyed; /* is currently being destroyed */
/* FIXME: OIDs should be given out by RPCSS */ OID oidc; /* object ID counter, starts at 1, zero is invalid OID (CS cs) */
On Wed, Mar 28, 2018 at 09:01:58PM -0500, Zebediah Figura wrote:
This will need some thought. I'll get back to you.
Huw.
On Thu, Mar 29, 2018 at 11:31:37AM +0100, Huw Davies wrote:
Ok, this looks reasonable. Thanks for bearing with me.
Signed-off-by: Huw Davies huw@codeweavers.com
-
Huw Davies -
Zebediah Figura