[PATCH 1/6] ole32/tests: Simplify implicit MTA tests.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 dlls/ole32/tests/compobj.c | 229 ++++++++++-----------------------------------
 1 file changed, 47 insertions(+), 182 deletions(-)

diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c
index f1e0134..da4223b 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,48 @@ 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;
+    CLSID clsid;
+    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 +3787,5 @@ START_TEST(compobj)
     test_IInitializeSpy();
     test_CoGetInstanceFromFile();
     test_GlobalOptions();
+    test_implicit_mta();
 }

[PATCH 2/6] ole32: Allow more functions to use the implicit MTA.

[Zebediah Figura]

It's necessary to introduce the being_destroyed field since a destroyed
apartment may end up releasing proxy objects, which will require calling
various functions, e.g. CoGetPSClsid(), potentially triggering an infinite
recursion otherwise.

Signed-off-by: Zebediah Figura 
---
 dlls/ole32/compobj.c         | 165 +++++++++++++++++++++++++------------------
 dlls/ole32/compobj_private.h |   1 +
 dlls/ole32/tests/compobj.c   |  23 ++++++
 3 files changed, 120 insertions(+), 69 deletions(-)

diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
index cd0e67f..61a8ae6 100644
--- a/dlls/ole32/compobj.c
+++ b/dlls/ole32/compobj.c
@@ -717,6 +717,44 @@ 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_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;
+}
+
+/* Return the current apartment if it exists, or, failing that, the MTA. Caller
+ * must free the returned apartment in either case. */
+static APARTMENT *apartment_get_current_or_mta(void)
+{
+    APARTMENT *apt = COM_CurrentApt();
+    if (apt)
+    {
+        apartment_addref(apt);
+        return apt;
+    }
+    return apartment_find_multi_threaded();
+}
+
 static void COM_RevokeRegisteredClassObject(RegisteredClass *curClass)
 {
     list_remove(&curClass->entry);
@@ -1059,8 +1097,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoRevokeClassObject(

TRACE("(%08x)\n",dwRegister);

-  apt = COM_CurrentApt();
-  if (!apt)
+  if (!(apt = apartment_get_current_or_mta()))
   {
     ERR("COM was not initialized\n");
     return CO_E_NOTINITIALIZED;
@@ -1091,7 +1128,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoRevokeClassObject(
   }

LeaveCriticalSection( &csRegisteredClassList );
-
+  apartment_release(apt);
   return hr;
 }

@@ -1145,9 +1182,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);
@@ -1295,31 +1340,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,
@@ -2059,9 +2079,11 @@ HRESULT WINAPI CoDisconnectObject( LPUNKNOWN lpUnk, DWORD reserved )
         return hr;
     }

-    apt = COM_CurrentApt();
-    if (!apt)
+    if (!(apt = apartment_get_current_or_mta()))
+    {
+        ERR("apartment not initialised\n");
         return CO_E_NOTINITIALIZED;
+    }

manager = get_stub_manager_from_object(apt, lpUnk, FALSE);
     if (manager) {
@@ -2076,6 +2098,7 @@ HRESULT WINAPI CoDisconnectObject( LPUNKNOWN lpUnk, DWORD reserved )
      * not found, making apps think that the object was disconnected, when
      * it actually wasn't */

+    apartment_release(apt);
     return S_OK;
 }

@@ -2583,7 +2606,7 @@ HRESULT WINAPI CoGetPSClsid(REFIID riid, CLSID *pclsid)
     static const WCHAR wszInterface[] = {'I','n','t','e','r','f','a','c','e','\',0};
     static const WCHAR wszPSC[] = {'\','P','r','o','x','y','S','t','u','b','C','l','s','i','d','3','2',0};
     WCHAR path[ARRAYSIZE(wszInterface) - 1 + CHARS_IN_GUID - 1 + ARRAYSIZE(wszPSC)];
-    APARTMENT *apt = COM_CurrentApt();
+    APARTMENT *apt;
     struct registered_psclsid *registered_psclsid;
     ACTCTX_SECTION_KEYED_DATA data;
     HRESULT hr;
@@ -2592,11 +2615,12 @@ HRESULT WINAPI CoGetPSClsid(REFIID riid, CLSID *pclsid)

TRACE("() riid=%s, pclsid=%p\n", debugstr_guid(riid), pclsid);

-    if (!apt)
+    if (!(apt = apartment_get_current_or_mta()))
     {
         ERR("apartment not initialised\n");
         return CO_E_NOTINITIALIZED;
     }
+    apartment_release(apt);

if (!pclsid)
         return E_INVALIDARG;
@@ -2667,16 +2691,17 @@ HRESULT WINAPI CoGetPSClsid(REFIID riid, CLSID *pclsid)
  */
 HRESULT WINAPI CoRegisterPSClsid(REFIID riid, REFCLSID rclsid)
 {
-    APARTMENT *apt = COM_CurrentApt();
+    APARTMENT *apt;
     struct registered_psclsid *registered_psclsid;

TRACE("(%s, %s)\n", debugstr_guid(riid), debugstr_guid(rclsid));

-    if (!apt)
+    if (!(apt = apartment_get_current_or_mta()))
     {
         ERR("apartment not initialised\n");
         return CO_E_NOTINITIALIZED;
     }
+    apartment_release(apt);

EnterCriticalSection(&cs_registered_psclsid_list);

@@ -2802,11 +2827,10 @@ HRESULT WINAPI CoRegisterClassObject(
   if ( (lpdwRegister==0) || (pUnk==0) )
     return E_INVALIDARG;

-  apt = COM_CurrentApt();
-  if (!apt)
+  if (!(apt = apartment_get_current_or_mta()))
   {
-      ERR("COM was not initialized\n");
-      return CO_E_NOTINITIALIZED;
+    ERR("apartment not initialised\n");
+    return CO_E_NOTINITIALIZED;
   }

*lpdwRegister = 0;
@@ -2826,16 +2850,21 @@ HRESULT WINAPI CoRegisterClassObject(
       if (dwClsContext & CLSCTX_LOCAL_SERVER)
         hr = CoLockObjectExternal(foundObject, TRUE, FALSE);
       IUnknown_Release(foundObject);
+      apartment_release(apt);
       return hr;
     }
     IUnknown_Release(foundObject);
     ERR("object already registered for class %s\n", debugstr_guid(rclsid));
+    apartment_release(apt);
     return CO_E_OBJISREG;
   }

newClass = HeapAlloc(GetProcessHeap(), 0, sizeof(RegisteredClass));
   if ( newClass == NULL )
+  {
+    apartment_release(apt);
     return E_OUTOFMEMORY;
+  }

newClass->classIdentifier = *rclsid;
   newClass->apartment_id    = apt->oxid;
@@ -2864,7 +2893,10 @@ HRESULT WINAPI CoRegisterClassObject(

hr = get_local_server_stream(apt, &marshal_stream);
       if(FAILED(hr))
+      {
+          apartment_release(apt);
           return hr;
+      }

hr = RPC_StartLocalServer(&newClass->classIdentifier,
                                 marshal_stream,
@@ -2872,6 +2904,7 @@ HRESULT WINAPI CoRegisterClassObject(
                                 &newClass->RpcRegistration);
       IStream_Release(marshal_stream);
   }
+  apartment_release(apt);
   return S_OK;
 }

@@ -2989,7 +3022,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));

@@ -2998,14 +3030,10 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(

*ppv = NULL;

-    if (!(apt = COM_CurrentApt()))
+    if (!(apt = apartment_get_current_or_mta()))
     {
-        if (!(apt = apartment_find_multi_threaded()))
-        {
-            ERR("apartment not initialised\n");
-            return CO_E_NOTINITIALIZED;
-        }
-        release_apt = TRUE;
+        ERR("apartment not initialised\n");
+        return CO_E_NOTINITIALIZED;
     }

if (pServerInfo) {
@@ -3017,7 +3045,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))
@@ -3043,7 +3071,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;
         }
     }
@@ -3064,7 +3092,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;
     }

@@ -3099,7 +3127,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
          * other types */
         if (SUCCEEDED(hres))
         {
-            if (release_apt) apartment_release(apt);
+            apartment_release(apt);
             return hres;
         }
     }
@@ -3135,11 +3163,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)
@@ -3298,15 +3326,12 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoCreateInstanceEx(
     if(FAILED(hres))
         clsid = *rclsid;

-    if (!(apt = COM_CurrentApt()))
+    if (!(apt = apartment_get_current_or_mta()))
     {
-        if (!(apt = apartment_find_multi_threaded()))
-        {
-            ERR("apartment not initialised\n");
-            return CO_E_NOTINITIALIZED;
-        }
-        apartment_release(apt);
+        ERR("apartment not initialised\n");
+        return CO_E_NOTINITIALIZED;
     }
+    apartment_release(apt);

/*
      * The Standard Global Interface Table (GIT) object is a process-wide singleton.
@@ -3640,8 +3665,11 @@ HRESULT WINAPI CoLockObjectExternal(
     TRACE("pUnk=%p, fLock=%s, fLastUnlockReleases=%s\n",
           pUnk, fLock ? "TRUE" : "FALSE", fLastUnlockReleases ? "TRUE" : "FALSE");

-    apt = COM_CurrentApt();
-    if (!apt) return CO_E_NOTINITIALIZED;
+    if (!(apt = apartment_get_current_or_mta()))
+    {
+        ERR("apartment not initialised\n");
+        return CO_E_NOTINITIALIZED;
+    }

stubmgr = get_stub_manager_from_object(apt, pUnk, fLock);
     if (!stubmgr)
@@ -3650,6 +3678,7 @@ HRESULT WINAPI CoLockObjectExternal(
         /* Note: native is pretty broken here because it just silently
          * fails, without returning an appropriate error code, making apps
          * think that the object was disconnected, when it actually wasn't */
+        apartment_release(apt);
         return S_OK;
     }

@@ -3659,6 +3688,7 @@ HRESULT WINAPI CoLockObjectExternal(
         stub_manager_ext_release(stubmgr, 1, FALSE, fLastUnlockReleases);

stub_manager_int_release(stubmgr);
+    apartment_release(apt);
     return S_OK;
 }

@@ -4998,22 +5028,19 @@ HRESULT WINAPI CoGetObjectContext(REFIID riid, void **ppv)
 HRESULT WINAPI CoGetContextToken( ULONG_PTR *token )
 {
     struct oletls *info = COM_CurrentInfo();
+    APARTMENT *apt;

TRACE("(%p)\n", token);

if (!info)
         return E_OUTOFMEMORY;

-    if (!info->apt)
+    if (!(apt = apartment_get_current_or_mta()))
     {
-        APARTMENT *apt;
-        if (!(apt = apartment_find_multi_threaded()))
-        {
-            ERR("apartment not initialised\n");
-            return CO_E_NOTINITIALIZED;
-        }
-        apartment_release(apt);
+        ERR("apartment not initialised\n");
+        return CO_E_NOTINITIALIZED;
     }
+    apartment_release(apt);

if (!token)
         return E_POINTER;
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) */
diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c
index da4223b..ecfbac8 100644
--- a/dlls/ole32/tests/compobj.c
+++ b/dlls/ole32/tests/compobj.c
@@ -3701,6 +3701,7 @@ static DWORD CALLBACK implicit_mta_proc(void *param)
     IComThreadingInfo *threading_info;
     ULONG_PTR token;
     IUnknown *unk;
+    DWORD cookie;
     CLSID clsid;
     HRESULT hr;

@@ -3721,6 +3722,28 @@ static DWORD CALLBACK implicit_mta_proc(void *param)
     hr = CoGetContextToken(&token);
     ok_ole_success(hr, "CoGetContextToken");

+    hr = CoRegisterPSClsid(&IID_IWineTest, &CLSID_WineTestPSFactoryBuffer);
+    ok_ole_success(hr, "CoRegisterPSClsid");
+
+    hr = CoGetPSClsid(&IID_IClassFactory, &clsid);
+    ok_ole_success(hr, "CoGetPSClsid");
+
+    hr = CoRegisterClassObject(&CLSID_WineOOPTest, (IUnknown *)&Test_ClassFactory,
+                               CLSCTX_INPROC_SERVER, REGCLS_SINGLEUSE, &cookie);
+    ok_ole_success(hr, "CoRegisterClassObject");
+
+    hr = CoRevokeClassObject(cookie);
+    ok_ole_success(hr, "CoRevokeClassObject");
+
+    hr = CoRegisterMessageFilter(NULL, NULL);
+    ok(hr == CO_E_NOT_SUPPORTED, "got %#x\n", hr);
+
+    hr = CoLockObjectExternal((IUnknown *)&Test_Unknown, TRUE, TRUE);
+    ok_ole_success(hr, "CoLockObjectExternal");
+
+    hr = CoDisconnectObject((IUnknown *)&Test_Unknown, 0);
+    ok_ole_success(hr, "CoDisconnectObject");
+
     return 0;
 }

[PATCH 2/6] ole32: Allow more functions to use the implicit MTA.

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?
Full results can be found at
https://testbot.winehq.org/JobDetails.pl?Key=37128

Your paranoid android.

=== w7u (32 bit compobj) ===
compobj.c:2938: Test failed: CoWaitForMultipleHandles didn't pump all WM_DDE_FIRST messages
compobj.c:2941: Test failed: PeekMessageA failed, error 1400
compobj.c:2942: Test failed: expected msg.message = WM_QUIT, got 0
compobj.c:2943: Test failed: expected msg.wParam = 42, got 0

[PATCH 2/6] ole32: Allow more functions to use the implicit MTA.

[Zebediah Figura]

On 25/03/18 12:42, Marvin wrote:
> Hi,
>
> While running your changed tests on Windows, I think I found new failures.
> Being a bot and all I'm not very good at pattern recognition, so I might be
> wrong, but could you please double-check?
> Full results can be found at
> https://testbot.winehq.org/JobDetails.pl?Key=37128
>
> Your paranoid android.
>
> === w7u (32 bit compobj) ===
> compobj.c:2938: Test failed: CoWaitForMultipleHandles didn't pump all WM_DDE_FIRST messages
> compobj.c:2941: Test failed: PeekMessageA failed, error 1400
> compobj.c:2942: Test failed: expected msg.message = WM_QUIT, got 0
> compobj.c:2943: Test failed: expected msg.wParam = 42, got 0

This is a preëxisting failure, and not one affected by my tests:

http://test.winehq.org/data/5946973021285dd6ecb8df224956fea4817f8fed/win10_c...

[PATCH 2/6] ole32: Allow more functions to use the implicit MTA.

[Huw Davies]

On Sun, Mar 25, 2018 at 12:33:58PM -0500, Zebediah Figura wrote:
> It's necessary to introduce the being_destroyed field since a destroyed
> apartment may end up releasing proxy objects, which will require calling
> various functions, e.g. CoGetPSClsid(), potentially triggering an infinite
> recursion otherwise.
> Signed-off-by: Zebediah Figura
>
> dlls/ole32/compobj.c         | 165 +++++++++++++++++++++++++------------------
>  dlls/ole32/compobj_private.h |   1 +
>  dlls/ole32/tests/compobj.c   |  23 ++++++
>  3 files changed, 120 insertions(+), 69 deletions(-)
>
> diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
> index cd0e67f..61a8ae6 100644
> --- a/dlls/ole32/compobj.c
> +++ b/dlls/ole32/compobj.c
> @@ -717,6 +717,44 @@ 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_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;
> -
>    }
> - }

I know this function was copied from below, but there's the MTA variable
(protected by csApartment) that holds the MTA if it exists.  So you
could simply return that rather than loop through every apt.
> -
> - LeaveCriticalSection(&csApartment);
> - return result;
>
> +}
> -
>
> +/* Return the current apartment if it exists, or, failing that, the MTA. Caller
> -
> - must free the returned apartment in either case. */
>
> +static APARTMENT *apartment_get_current_or_mta(void)
> +{
> - APARTMENT *apt = COM_CurrentApt();
> - if (apt)
> - {
> -
>    apartment_addref(apt);
> -
>    return apt;
> - }
> - return apartment_find_multi_threaded();
>
> +}

So this patch is doing a couple of things.  It's taking a ref on the
current apt if one exists or using the MTA if it doesn't.  Since this
code is so fragile, I'd like these split.  Something like this would
work:

2.1 Add apartment_addref()s / apartment_release()s to CoGetClassObject(),
CoLockObjectExternal(), etc.  i.e. implement the 'taking a ref on the
current apt' bit.

2.2 Move apartment_find_multi_threaded() higher up in the file and rewrite
using the MTA variable.  Also why not rename this to apartment_find_mta()

2.3 Add apartment_get_current_or_mta() and use it in CoGetClassObject() etc.

Also, I'm not yet 100% convinced about the being_destroyed thing.  I'm
wondering whether there's a better way of doing that.  Hopefully
splitting this up will help with that.

Huw.
> static void COM_RevokeRegisteredClassObject(RegisteredClass *curClass)
>  {
>      list_remove(&curClass->entry);
> @@ -1059,8 +1097,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoRevokeClassObject(
>
> TRACE("(%08x)\n",dwRegister);
> - apt = COM_CurrentApt();
> - if (!apt)
> - if (!(apt = apartment_get_current_or_mta()))
> {
>   ERR("COM was not initialized\n");
>   return CO_E_NOTINITIALIZED;
>
> @@ -1091,7 +1128,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoRevokeClassObject(
>    }
>
> LeaveCriticalSection( &csRegisteredClassList );
> -
> - apartment_release(apt);
> return hr;
>
> }
>
> @@ -1145,9 +1182,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);
>
> @@ -1295,31 +1340,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,
> @@ -2059,9 +2079,11 @@ HRESULT WINAPI CoDisconnectObject( LPUNKNOWN lpUnk, DWORD reserved )
>          return hr;
>      }
> - apt = COM_CurrentApt();
> - if (!apt)
> -
>
> if (!(apt = apartment_get_current_or_mta()))
> -
>
> {
> -
>    ERR("apartment not initialised\n");
>    return CO_E_NOTINITIALIZED;
> -
>
> }
>
> manager = get_stub_manager_from_object(apt, lpUnk, FALSE);
> if (manager) {
>
> @@ -2076,6 +2098,7 @@ HRESULT WINAPI CoDisconnectObject( LPUNKNOWN lpUnk, DWORD reserved )
>       * not found, making apps think that the object was disconnected, when
>       * it actually wasn't */
> - apartment_release(apt);
> return S_OK;
>
> }
>
> @@ -2583,7 +2606,7 @@ HRESULT WINAPI CoGetPSClsid(REFIID riid, CLSID *pclsid)
>      static const WCHAR wszInterface[] = {'I','n','t','e','r','f','a','c','e','\',0};
>      static const WCHAR wszPSC[] = {'\','P','r','o','x','y','S','t','u','b','C','l','s','i','d','3','2',0};
>      WCHAR path[ARRAYSIZE(wszInterface) - 1 + CHARS_IN_GUID - 1 + ARRAYSIZE(wszPSC)];
> - APARTMENT *apt = COM_CurrentApt();
> - APARTMENT *apt;
> struct registered_psclsid *registered_psclsid;
> ACTCTX_SECTION_KEYED_DATA data;
> HRESULT hr;
>
> @@ -2592,11 +2615,12 @@ HRESULT WINAPI CoGetPSClsid(REFIID riid, CLSID *pclsid)
>  TRACE("() riid=%s, pclsid=%p\n", debugstr_guid(riid), pclsid);
>
> - if (!apt)
> -
>
> if (!(apt = apartment_get_current_or_mta()))
> {
>     ERR("apartment not initialised\n");
>     return CO_E_NOTINITIALIZED;
> }
> -
>
> apartment_release(apt);
>
> if (!pclsid)
>     return E_INVALIDARG;
>
> @@ -2667,16 +2691,17 @@ HRESULT WINAPI CoGetPSClsid(REFIID riid, CLSID *pclsid)
>   */
>  HRESULT WINAPI CoRegisterPSClsid(REFIID riid, REFCLSID rclsid)
>  {
> - APARTMENT *apt = COM_CurrentApt();
> -
>
> APARTMENT *apt;
> struct registered_psclsid *registered_psclsid;
>
> TRACE("(%s, %s)\n", debugstr_guid(riid), debugstr_guid(rclsid));
> - if (!apt)
> -
>
> if (!(apt = apartment_get_current_or_mta()))
> {
>     ERR("apartment not initialised\n");
>     return CO_E_NOTINITIALIZED;
> }
> -
>
> apartment_release(apt);
>
> EnterCriticalSection(&cs_registered_psclsid_list);
>
> @@ -2802,11 +2827,10 @@ HRESULT WINAPI CoRegisterClassObject(
>    if ( (lpdwRegister==0) || (pUnk==0) )
>      return E_INVALIDARG;
> - apt = COM_CurrentApt();
> - if (!apt)
> - if (!(apt = apartment_get_current_or_mta()))
> {
> -
>  ERR("COM was not initialized\n");
> -
>  return CO_E_NOTINITIALIZED;
> -
>
> ERR("apartment not initialised\n");
> -
>
> return CO_E_NOTINITIALIZED;
> }
>
> *lpdwRegister = 0;
>
> @@ -2826,16 +2850,21 @@ HRESULT WINAPI CoRegisterClassObject(
>        if (dwClsContext & CLSCTX_LOCAL_SERVER)
>          hr = CoLockObjectExternal(foundObject, TRUE, FALSE);
>        IUnknown_Release(foundObject);
> -
>  apartment_release(apt);
>  return hr;
>
> }
>    IUnknown_Release(foundObject);
>    ERR("object already registered for class %s\n", debugstr_guid(rclsid));
> -
>
> apartment_release(apt);
> return CO_E_OBJISREG;
> }
>
> newClass = HeapAlloc(GetProcessHeap(), 0, sizeof(RegisteredClass));
> if ( newClass == NULL )
> -
>
> {
> -
>
> apartment_release(apt);
> return E_OUTOFMEMORY;
> -
>
> }
>
> newClass->classIdentifier = *rclsid;
> newClass->apartment_id    = apt->oxid;
>
> @@ -2864,7 +2893,10 @@ HRESULT WINAPI CoRegisterClassObject(
>    hr = get_local_server_stream(apt, &marshal_stream);
>    if(FAILED(hr))
> -
>  {
> -
>      apartment_release(apt);
>      return hr;
> -
>  }
>
>  hr = RPC_StartLocalServer(&newClass->classIdentifier,
>                            marshal_stream,
>
> @@ -2872,6 +2904,7 @@ HRESULT WINAPI CoRegisterClassObject(
>                                  &newClass->RpcRegistration);
>        IStream_Release(marshal_stream);
>    }
> - apartment_release(apt);
> return S_OK;
>
> }
>
> @@ -2989,7 +3022,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));
>
> @@ -2998,14 +3030,10 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
>  *ppv = NULL;
>
> - if (!(apt = COM_CurrentApt()))
> - if (!(apt = apartment_get_current_or_mta()))
> {
> -
>    if (!(apt = apartment_find_multi_threaded()))
> -
>    {
> -
>        ERR("apartment not initialised\n");
> -
>        return CO_E_NOTINITIALIZED;
> -
>    }
> -
>    release_apt = TRUE;
> -
>    ERR("apartment not initialised\n");
> -
>    return CO_E_NOTINITIALIZED;
>
> }
>
> if (pServerInfo) {
>
> @@ -3017,7 +3045,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))
>
> @@ -3043,7 +3071,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;
>    }
> }
>
> @@ -3064,7 +3092,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;
> }
>
> @@ -3099,7 +3127,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoGetClassObject(
>           * other types */
>          if (SUCCEEDED(hres))
>          {
> -
>        if (release_apt) apartment_release(apt);
> -
>        apartment_release(apt);
>        return hres;
>    }
> }
>
> @@ -3135,11 +3163,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)
>
> @@ -3298,15 +3326,12 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CoCreateInstanceEx(
>      if(FAILED(hres))
>          clsid = *rclsid;
> - if (!(apt = COM_CurrentApt()))
> - if (!(apt = apartment_get_current_or_mta()))
> {
> -
>    if (!(apt = apartment_find_multi_threaded()))
> -
>    {
> -
>        ERR("apartment not initialised\n");
> -
>        return CO_E_NOTINITIALIZED;
> -
>    }
> -
>    apartment_release(apt);
> -
>    ERR("apartment not initialised\n");
> -
>    return CO_E_NOTINITIALIZED;
>
> }
> -
>
> apartment_release(apt);
>
> /*
> - The Standard Global Interface Table (GIT) object is a process-wide singleton.
>
> @@ -3640,8 +3665,11 @@ HRESULT WINAPI CoLockObjectExternal(
>      TRACE("pUnk=%p, fLock=%s, fLastUnlockReleases=%s\n",
>            pUnk, fLock ? "TRUE" : "FALSE", fLastUnlockReleases ? "TRUE" : "FALSE");
> - apt = COM_CurrentApt();
> - if (!apt) return CO_E_NOTINITIALIZED;
> -
>
> if (!(apt = apartment_get_current_or_mta()))
> -
>
> {
> -
>    ERR("apartment not initialised\n");
> -
>    return CO_E_NOTINITIALIZED;
> -
>
> }
>
> stubmgr = get_stub_manager_from_object(apt, pUnk, fLock);
> if (!stubmgr)
>
> @@ -3650,6 +3678,7 @@ HRESULT WINAPI CoLockObjectExternal(
>          /* Note: native is pretty broken here because it just silently
>           * fails, without returning an appropriate error code, making apps
>           * think that the object was disconnected, when it actually wasn't */
> -
>    apartment_release(apt);
>    return S_OK;
> }
>
> @@ -3659,6 +3688,7 @@ HRESULT WINAPI CoLockObjectExternal(
>          stub_manager_ext_release(stubmgr, 1, FALSE, fLastUnlockReleases);
>  stub_manager_int_release(stubmgr);
> - apartment_release(apt);
> return S_OK;
>
> }
>
> @@ -4998,22 +5028,19 @@ HRESULT WINAPI CoGetObjectContext(REFIID riid, void **ppv)
>  HRESULT WINAPI CoGetContextToken( ULONG_PTR *token )
>  {
>      struct oletls *info = COM_CurrentInfo();
> -
>
> APARTMENT *apt;
>
> TRACE("(%p)\n", token);
>
> if (!info)
>     return E_OUTOFMEMORY;
> - if (!info->apt)
> - if (!(apt = apartment_get_current_or_mta()))
> {
> -
>    APARTMENT *apt;
> -
>    if (!(apt = apartment_find_multi_threaded()))
> -
>    {
> -
>        ERR("apartment not initialised\n");
> -
>        return CO_E_NOTINITIALIZED;
> -
>    }
> -
>    apartment_release(apt);
> -
>    ERR("apartment not initialised\n");
> -
>    return CO_E_NOTINITIALIZED;
>
> }
> -
>
> apartment_release(apt);
>
> if (!token)
>     return E_POINTER;
>
> 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) */
>
> diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c
> index da4223b..ecfbac8 100644
> --- a/dlls/ole32/tests/compobj.c
> +++ b/dlls/ole32/tests/compobj.c
> @@ -3701,6 +3701,7 @@ static DWORD CALLBACK implicit_mta_proc(void *param)
>      IComThreadingInfo *threading_info;
>      ULONG_PTR token;
>      IUnknown *unk;
> - DWORD cookie;
> CLSID clsid;
> HRESULT hr;
>
> @@ -3721,6 +3722,28 @@ static DWORD CALLBACK implicit_mta_proc(void *param)
>      hr = CoGetContextToken(&token);
>      ok_ole_success(hr, "CoGetContextToken");
> - hr = CoRegisterPSClsid(&IID_IWineTest, &CLSID_WineTestPSFactoryBuffer);
> - ok_ole_success(hr, "CoRegisterPSClsid");
> -
> - hr = CoGetPSClsid(&IID_IClassFactory, &clsid);
> - ok_ole_success(hr, "CoGetPSClsid");
> -
> - hr = CoRegisterClassObject(&CLSID_WineOOPTest, (IUnknown *)&Test_ClassFactory,
> -
>                           CLSCTX_INPROC_SERVER, REGCLS_SINGLEUSE, &cookie);
> - ok_ole_success(hr, "CoRegisterClassObject");
> -
> - hr = CoRevokeClassObject(cookie);
> - ok_ole_success(hr, "CoRevokeClassObject");
> -
> - hr = CoRegisterMessageFilter(NULL, NULL);
> - ok(hr == CO_E_NOT_SUPPORTED, "got %#x\n", hr);
> -
> - hr = CoLockObjectExternal((IUnknown *)&Test_Unknown, TRUE, TRUE);
> - ok_ole_success(hr, "CoLockObjectExternal");
> -
> - hr = CoDisconnectObject((IUnknown *)&Test_Unknown, 0);
> - ok_ole_success(hr, "CoDisconnectObject");
> - return 0;
>
> }
>
> -- 
> 2.7.4

[PATCH 2/6] ole32: Allow more functions to use the implicit MTA.

[Zebediah Figura]

On 28/03/18 03:43, Huw Davies wrote:
> On Sun, Mar 25, 2018 at 12:33:58PM -0500, Zebediah Figura wrote:
> > It's necessary to introduce the being_destroyed field since a destroyed
> > apartment may end up releasing proxy objects, which will require calling
> > various functions, e.g. CoGetPSClsid(), potentially triggering an infinite
> > recursion otherwise.
> > Signed-off-by: Zebediah Figura
> >
> > dlls/ole32/compobj.c         | 165 +++++++++++++++++++++++++------------------
> >  dlls/ole32/compobj_private.h |   1 +
> >  dlls/ole32/tests/compobj.c   |  23 ++++++
> >  3 files changed, 120 insertions(+), 69 deletions(-)
> >
> > diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
> > index cd0e67f..61a8ae6 100644
> > --- a/dlls/ole32/compobj.c
> > +++ b/dlls/ole32/compobj.c
> > @@ -717,6 +717,44 @@ 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_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;
> > -
> >    }
> > - }
>
> I know this function was copied from below, but there's the MTA variable
> (protected by csApartment) that holds the MTA if it exists.  So you
> could simply return that rather than loop through every apt.
> > -
> > - LeaveCriticalSection(&csApartment);
> > - return result;
> >
> > +}
> > -
> >
> > +/* Return the current apartment if it exists, or, failing that, the MTA. Caller
> > -
> > - must free the returned apartment in either case. */
> >
> > +static APARTMENT *apartment_get_current_or_mta(void)
> > +{
> > - APARTMENT *apt = COM_CurrentApt();
> > - if (apt)
> > - {
> > -
> >    apartment_addref(apt);
> > -
> >    return apt;
> > - }
> > - return apartment_find_multi_threaded();
> >
> > +}
>
> So this patch is doing a couple of things.  It's taking a ref on the
> current apt if one exists or using the MTA if it doesn't.  Since this
> code is so fragile, I'd like these split.  Something like this would
> work:
>
> 2.1 Add apartment_addref()s / apartment_release()s to CoGetClassObject(),
> CoLockObjectExternal(), etc.  i.e. implement the 'taking a ref on the
> current apt' bit.
>
> 2.2 Move apartment_find_multi_threaded() higher up in the file and rewrite
> using the MTA variable.  Also why not rename this to apartment_find_mta()
>
> 2.3 Add apartment_get_current_or_mta() and use it in CoGetClassObject() etc.
>
> Also, I'm not yet 100% convinced about the being_destroyed thing.  I'm
> wondering whether there's a better way of doing that.  Hopefully
> splitting this up will help with that.
>
> Huw.

Thanks, that sounds like a good idea.

I think I considered locking the apartment using its critical section
instead and didn't due to concerns with marshalling, but I'll take a
closer look and see if that should be possible.

[PATCH 3/6] ole32: Report the implicit MTA in CoGetApartmentType().

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 dlls/ole32/compobj.c       |  8 ++++++++
 dlls/ole32/tests/compobj.c | 19 ++++++++-----------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
index 61a8ae6..548565d 100644
--- a/dlls/ole32/compobj.c
+++ b/dlls/ole32/compobj.c
@@ -5116,6 +5116,7 @@ HRESULT Handler_DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID *ppv)
 HRESULT WINAPI CoGetApartmentType(APTTYPE *type, APTTYPEQUALIFIER *qualifier)
 {
     struct oletls *info = COM_CurrentInfo();
+    APARTMENT *apt;

FIXME("(%p, %p): semi-stub\n", type, qualifier);

@@ -5136,6 +5137,13 @@ HRESULT WINAPI CoGetApartmentType(APTTYPE *type, APTTYPEQUALIFIER *qualifier)

*qualifier = APTTYPEQUALIFIER_NONE;

+    if (!info->apt && (apt = apartment_find_multi_threaded()))
+    {
+        apartment_release(apt);
+        *type = APTTYPE_MTA;
+        *qualifier = APTTYPEQUALIFIER_IMPLICIT_MTA;
+    }
+
     return info->apt ? S_OK : CO_E_NOTINITIALIZED;
 }

diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c
index ecfbac8..b9b7be0 100644
--- a/dlls/ole32/tests/compobj.c
+++ b/dlls/ole32/tests/compobj.c
@@ -602,9 +602,8 @@ static void test_StringFromGUID2(void)
   ok(len == 0, "len: %d (expected 0)\n", len);
 }

-#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)
+#define test_apt_type(t, q) _test_apt_type(t, q, __LINE__)
+static void _test_apt_type(APTTYPE expected_type, APTTYPEQUALIFIER expected_qualifier, int line)
 {
     APTTYPEQUALIFIER qualifier = ~0u;
     APTTYPE type = ~0u;
@@ -615,9 +614,7 @@ static void _test_apt_type(APTTYPE expected_type, APTTYPEQUALIFIER expected_qual

hr = pCoGetApartmentType(&type, &qualifier);
     ok_(__FILE__, line)(hr == S_OK || hr == CO_E_NOTINITIALIZED, "Unexpected return code: 0x%08x\n", hr);
-todo_wine_if(todo_type)
     ok_(__FILE__, line)(type == expected_type, "Wrong apartment type %d, expected %d\n", type, expected_type);
-todo_wine_if(todo_qualifier)
     ok_(__FILE__, line)(qualifier == expected_qualifier, "Wrong apartment qualifier %d, expected %d\n", qualifier,
         expected_qualifier);
 }
@@ -660,7 +657,7 @@ 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);

-    test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE);
+    test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE);
 }

static void test_CoGetClassObject(void)
@@ -682,7 +679,7 @@ static void test_CoGetClassObject(void)
        broken(hr == CO_E_NOTINITIALIZED), /* win9x */
        "CoGetClassObject should have returned E_INVALIDARG instead of 0x%08x\n", hr);

-    test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE);
+    test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE);

if (!pRegOverridePredefKey)
     {
@@ -1797,7 +1794,7 @@ static void test_CoGetObjectContext(void)

pCoInitializeEx(NULL, COINIT_APARTMENTTHREADED);

-    test_apt_type(APTTYPE_MAINSTA, APTTYPEQUALIFIER_NONE, FALSE, FALSE);
+    test_apt_type(APTTYPE_MAINSTA, APTTYPEQUALIFIER_NONE);

hr = pCoGetObjectContext(&IID_IComThreadingInfo, (void **)&pComThreadingInfo);
     ok_ole_success(hr, "CoGetObjectContext");
@@ -1993,11 +1990,11 @@ 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);

-    test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE, FALSE, FALSE);
+    test_apt_type(APTTYPE_CURRENT, APTTYPEQUALIFIER_NONE);

CoInitialize(NULL);

-    test_apt_type(APTTYPE_MAINSTA, APTTYPEQUALIFIER_NONE, FALSE, FALSE);
+    test_apt_type(APTTYPE_MAINSTA, APTTYPEQUALIFIER_NONE);

hr = pCoGetContextToken(NULL);
     ok(hr == E_POINTER, "Expected E_POINTER, got 0x%08x\n", hr);
@@ -3705,7 +3702,7 @@ static DWORD CALLBACK implicit_mta_proc(void *param)
     CLSID clsid;
     HRESULT hr;

-    test_apt_type(APTTYPE_MTA, APTTYPEQUALIFIER_IMPLICIT_MTA, TRUE, TRUE);
+    test_apt_type(APTTYPE_MTA, APTTYPEQUALIFIER_IMPLICIT_MTA);

hr = CoCreateInstance(&CLSID_InternetZoneManager, NULL, CLSCTX_INPROC_SERVER, &IID_IUnknown, (void **)&unk);
     ok_ole_success(hr, "CoCreateInstance");

[PATCH 4/6] ole32/tests: Test CoWaitForMultipleHandles() in an uninitialized or MTA apartment.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 dlls/ole32/tests/compobj.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c
index b9b7be0..c43e86c 100644
--- a/dlls/ole32/tests/compobj.c
+++ b/dlls/ole32/tests/compobj.c
@@ -2998,14 +2998,36 @@ static void test_CoWaitForMultipleHandles(void)
     ok(index == WAIT_OBJECT_0, "WaitForSingleObject failed\n");
     CloseHandle(thread);

+    CoUninitialize();
+
+    /* If COM was not initialized, messages are neither pumped nor peeked at */
+    PostMessageA(hWnd, WM_DDE_FIRST, 0, 0);
+    hr = CoWaitForMultipleHandles(0, 100, 2, handles, &index);
+    ok(hr == RPC_S_CALLPENDING, "got %#x\n", hr);
+    success = MsgWaitForMultipleObjectsEx(0, NULL, 2, QS_ALLPOSTMESSAGE, MWMO_ALERTABLE);
+    ok(success == 0, "MsgWaitForMultipleObjects returned %x\n", success);
+    success = PeekMessageA(&msg, hWnd, WM_DDE_FIRST, WM_DDE_FIRST, PM_REMOVE);
+    ok(success, "PeekMessage failed: %u\n", GetLastError());
+
+    /* same in an MTA */
+    CoInitializeEx(NULL, COINIT_MULTITHREADED);
+
+    PostMessageA(hWnd, WM_DDE_FIRST, 0, 0);
+    hr = CoWaitForMultipleHandles(0, 100, 2, handles, &index);
+    ok(hr == RPC_S_CALLPENDING, "got %#x\n", hr);
+    success = MsgWaitForMultipleObjectsEx(0, NULL, 2, QS_ALLPOSTMESSAGE, MWMO_ALERTABLE);
+    ok(success == 0, "MsgWaitForMultipleObjects returned %x\n", success);
+    success = PeekMessageA(&msg, hWnd, WM_DDE_FIRST, WM_DDE_FIRST, PM_REMOVE);
+    ok(success, "PeekMessage failed: %u\n", GetLastError());
+
+    CoUninitialize();
+
     CloseHandle(handles[0]);
     CloseHandle(handles[1]);
     DestroyWindow(hWnd);

success = UnregisterClassA(cls_name, GetModuleHandleA(0));
     ok(success, "UnregisterClass failed %u\n", GetLastError());
-
-    CoUninitialize();
 }

static void test_CoGetMalloc(void)

[PATCH 5/6] ole32: Allow unmarshalling objects into an implicit MTA.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
This patch results in some good news and some bad news for bug 18070.

The good news is that we can unmarshal into literally any
thread—uninitialized, STA, or MTA—as long as there's an MTA somewhere
else in the process. This means we can use a COM proxy on any
thread. The bad news is that we can't change threading models
(i.e. initialize or uninitialize COM on a thread) while we're doing
this, or ole32 will throw up RPC_E_WRONG_THREAD when we try to make a
proxy call.

The symptoms of bug 18070 show that there has to exist an MTA in the
process, but that COM can't be initialized on the thread itself: "It
is up to the custom action to initialize COM to its liking." This fits
the MTA condition quite nicely, but then runs into the separate
problem that the custom action can change the threading model on us.

What this means is that we can't keep proxies around on the custom
action's thread and then just run all calls to MSI functions through
them, since they might go bad at any point. I see at least two
solutions to this:

(1) Keep the COM objects on a different thread (probably just the
thread that the MTA is initialized on). Then use a dedicated 'pipe'
thread to thunk callbacks to the MTA thread, whence they will then be
converted into proxy calls.

(2) Instead of keeping COM objects around during the entire custom
action, create and then release one generic 'server' object every time
an MSI function is called. This will, of course, work regardless of
what threading model the custom action's thread is actively using,
since it won't change during the course of the call.

(3) Along the same lines as (2), but potentially even simpler: use the
RPC APIs directly. That is, replace our
IWineMsiRemotePackage_GetProperty(...) with a simpler
remote_GetProperty(...) that does about the same thing, but without
the overhead of going through ole32. (And for all the work I did to
make ole32 work right! But it is for the best.)

I'm most inclined towards (3)—I think it'll be the solution with the
least overhead, and it won't take too much work to convert from what
we have.

Therefore: if anyone has any alternate preferences, reasons why one or
more of the above won't work, or general feedback, please speak now.
Alternatively, wait until I've done all the work to convert everything
to RPC server stubs, and then tell me why it all won't work ;-)

dlls/ole32/compobj.c         |   2 +-
 dlls/ole32/compobj_private.h |   3 +-
 dlls/ole32/marshal.c         |  32 ++++++++----
 dlls/ole32/rpc.c             |  12 +++--
 dlls/ole32/tests/marshal.c   | 114 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 148 insertions(+), 15 deletions(-)

diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
index 548565d..93df2f4 100644
--- a/dlls/ole32/compobj.c
+++ b/dlls/ole32/compobj.c
@@ -744,7 +744,7 @@ static APARTMENT *apartment_find_multi_threaded(void)

/* Return the current apartment if it exists, or, failing that, the MTA. Caller
  * must free the returned apartment in either case. */
-static APARTMENT *apartment_get_current_or_mta(void)
+APARTMENT *apartment_get_current_or_mta(void)
 {
     APARTMENT *apt = COM_CurrentApt();
     if (apt)
diff --git a/dlls/ole32/compobj_private.h b/dlls/ole32/compobj_private.h
index 12413f7..dc09d20 100644
--- a/dlls/ole32/compobj_private.h
+++ b/dlls/ole32/compobj_private.h
@@ -212,7 +212,7 @@ void    RPC_StartRemoting(struct apartment *apt) DECLSPEC_HIDDEN;
 HRESULT RPC_CreateClientChannel(const OXID *oxid, const IPID *ipid,
                                 const OXID_INFO *oxid_info,
                                 DWORD dest_context, void *dest_context_data,
-                                IRpcChannelBuffer **chan) DECLSPEC_HIDDEN;
+                                IRpcChannelBuffer **chan, APARTMENT *apt) DECLSPEC_HIDDEN;
 HRESULT RPC_CreateServerChannel(DWORD dest_context, void *dest_context_data, IRpcChannelBuffer **chan) DECLSPEC_HIDDEN;
 void    RPC_ExecuteCall(struct dispatch_params *params) DECLSPEC_HIDDEN;
 HRESULT RPC_RegisterInterface(REFIID riid) DECLSPEC_HIDDEN;
@@ -248,6 +248,7 @@ HRESULT apartment_createwindowifneeded(struct apartment *apt) DECLSPEC_HIDDEN;
 HWND apartment_getwindow(const struct apartment *apt) DECLSPEC_HIDDEN;
 HRESULT enter_apartment(struct oletls *info, DWORD model) DECLSPEC_HIDDEN;
 void leave_apartment(struct oletls *info) DECLSPEC_HIDDEN;
+APARTMENT *apartment_get_current_or_mta(void) DECLSPEC_HIDDEN;

/* DCOM messages used by the apartment window (not compatible with native) */
 #define DM_EXECUTERPC   (WM_USER + 0) /* WPARAM = 0, LPARAM = (struct dispatch_params *) */
diff --git a/dlls/ole32/marshal.c b/dlls/ole32/marshal.c
index b39dac0..822d06e 100644
--- a/dlls/ole32/marshal.c
+++ b/dlls/ole32/marshal.c
@@ -313,13 +313,15 @@ static HRESULT WINAPI ClientIdentity_QueryMultipleInterfaces(IMultiQI *iface, UL
          * the interfaces were returned */
         if (SUCCEEDED(hr))
         {
+            APARTMENT *apt = apartment_get_current_or_mta();
+
             /* try to unmarshal each object returned to us */
             for (i = 0; i < nonlocal_mqis; i++)
             {
                 ULONG index = mapping[i];
                 HRESULT hrobj = qiresults[i].hResult;
                 if (hrobj == S_OK)
-                    hrobj = unmarshal_object(&qiresults[i].std, COM_CurrentApt(),
+                    hrobj = unmarshal_object(&qiresults[i].std, apt,
                                              This->dest_context,
                                              This->dest_context_data,
                                              pMQIs[index].pIID, &This->oxid_info,
@@ -331,6 +333,8 @@ static HRESULT WINAPI ClientIdentity_QueryMultipleInterfaces(IMultiQI *iface, UL
                     ERR("Failed to get pointer to interface %s\n", debugstr_guid(pMQIs[index].pIID));
                 pMQIs[index].hr = hrobj;
             }
+
+            apartment_release(apt);
         }

/* free the memory allocated by the proxy */
@@ -1010,8 +1014,7 @@ static HRESULT proxy_manager_get_remunknown(struct proxy_manager * This, IRemUnk
     if (This->sorflags & SORFP_NOLIFETIMEMGMT)
         return S_FALSE;

-    apt = COM_CurrentApt();
-    if (!apt)
+    if (!(apt = apartment_get_current_or_mta()))
         return CO_E_NOTINITIALIZED;

called_in_original_apt = This->parent && (This->parent->oxid == apt->oxid);
@@ -1046,7 +1049,7 @@ static HRESULT proxy_manager_get_remunknown(struct proxy_manager * This, IRemUnk
         stdobjref.ipid = This->oxid_info.ipidRemUnknown;

/* do the unmarshal */
-        hr = unmarshal_object(&stdobjref, COM_CurrentApt(), This->dest_context,
+        hr = unmarshal_object(&stdobjref, apt, This->dest_context,
                               This->dest_context_data, &IID_IRemUnknown,
                               &This->oxid_info, (void**)remunk);
         if (hr == S_OK && called_in_original_apt)
@@ -1056,6 +1059,7 @@ static HRESULT proxy_manager_get_remunknown(struct proxy_manager * This, IRemUnk
         }
     }
     LeaveCriticalSection(&This->cs);
+    apartment_release(apt);

TRACE("got IRemUnknown* pointer %p, hr = 0x%08x\n", *remunk, hr);

@@ -1288,7 +1292,7 @@ static HRESULT unmarshal_object(const STDOBJREF *stdobjref, APARTMENT *apt,
                                          &proxy_manager->oxid_info,
                                          proxy_manager->dest_context,
                                          proxy_manager->dest_context_data,
-                                         &chanbuf);
+                                         &chanbuf, apt);
             if (hr == S_OK)
                 hr = proxy_manager_create_ifproxy(proxy_manager, stdobjref,
                                                   riid, chanbuf, &ifproxy);
@@ -1324,14 +1328,14 @@ StdMarshalImpl_UnmarshalInterface(IMarshal *iface, IStream *pStm, REFIID riid, v
     STDOBJREF stdobjref;
     ULONG res;
     HRESULT hres;
-    APARTMENT *apt = COM_CurrentApt();
+    APARTMENT *apt;
     APARTMENT *stub_apt;
     OXID oxid;

TRACE("(...,%s,....)\n", debugstr_guid(riid));

/* we need an apartment to unmarshal into */
-    if (!apt)
+    if (!(apt = apartment_get_current_or_mta()))
     {
         ERR("Apartment not initialized\n");
         return CO_E_NOTINITIALIZED;
@@ -1339,10 +1343,18 @@ StdMarshalImpl_UnmarshalInterface(IMarshal *iface, IStream *pStm, REFIID riid, v

/* read STDOBJREF from wire */
     hres = IStream_Read(pStm, &stdobjref, sizeof(stdobjref), &res);
-    if (hres != S_OK) return STG_E_READFAULT;
+    if (hres != S_OK)
+    {
+        apartment_release(apt);
+        return STG_E_READFAULT;
+    }

hres = apartment_getoxid(apt, &oxid);
-    if (hres != S_OK) return hres;
+    if (hres != S_OK)
+    {
+        apartment_release(apt);
+        return hres;
+    }

/* check if we're marshalling back to ourselves */
     if ((oxid == stdobjref.oxid) && (stubmgr = get_stub_manager(apt, stdobjref.oid)))
@@ -1357,6 +1369,7 @@ StdMarshalImpl_UnmarshalInterface(IMarshal *iface, IStream *pStm, REFIID riid, v
             stub_manager_ext_release(stubmgr, stdobjref.cPublicRefs, stdobjref.flags & SORFP_TABLEWEAK, FALSE);

stub_manager_int_release(stubmgr);
+        apartment_release(apt);
         return hres;
     }

@@ -1395,6 +1408,7 @@ StdMarshalImpl_UnmarshalInterface(IMarshal *iface, IStream *pStm, REFIID riid, v
     if (hres != S_OK) WARN("Failed with error 0x%08x\n", hres);
     else TRACE("Successfully created proxy %p\n", *ppv);

+    apartment_release(apt);
     return hres;
 }

diff --git a/dlls/ole32/rpc.c b/dlls/ole32/rpc.c
index 8d8276e..5a7626b 100644
--- a/dlls/ole32/rpc.c
+++ b/dlls/ole32/rpc.c
@@ -830,14 +830,16 @@ static HRESULT WINAPI ClientRpcChannelBuffer_SendReceive(LPRPCCHANNELBUFFER ifac
     ORPC_EXTENT_ARRAY orpc_ext_array;
     WIRE_ORPC_EXTENT *first_wire_orpc_extent = NULL;
     HRESULT hrFault = S_OK;
+    APARTMENT *apt = apartment_get_current_or_mta();

TRACE("(%p) iMethod=%d\n", olemsg, olemsg->iMethod);

-    hr = ClientRpcChannelBuffer_IsCorrectApartment(This, COM_CurrentApt());
+    hr = ClientRpcChannelBuffer_IsCorrectApartment(This, apt);
     if (hr != S_OK)
     {
         ERR("called from wrong apartment, should have been 0x%s\n",
             wine_dbgstr_longlong(This->oxid));
+        if (apt) apartment_release(apt);
         return RPC_E_WRONG_THREAD;
     }
     /* This situation should be impossible in multi-threaded apartments,
@@ -845,11 +847,12 @@ static HRESULT WINAPI ClientRpcChannelBuffer_SendReceive(LPRPCCHANNELBUFFER ifac
      * Note: doing a COM call during the processing of a sent message is
      * only disallowed if a client call is already being waited for
      * completion */
-    if (!COM_CurrentApt()->multi_threaded &&
+    if (!apt->multi_threaded &&
         COM_CurrentInfo()->pending_call_count_client &&
         InSendMessage())
     {
         ERR("can't make an outgoing COM call in response to a sent message\n");
+        apartment_release(apt);
         return RPC_E_CANTCALLOUT_ININPUTSYNCCALL;
     }

@@ -967,6 +970,7 @@ static HRESULT WINAPI ClientRpcChannelBuffer_SendReceive(LPRPCCHANNELBUFFER ifac

TRACE("-- 0x%08x\n", hr);

+    apartment_release(apt);
     return hr;
 }

@@ -1094,7 +1098,7 @@ static const IRpcChannelBufferVtbl ServerRpcChannelBufferVtbl =
 HRESULT RPC_CreateClientChannel(const OXID *oxid, const IPID *ipid,
                                 const OXID_INFO *oxid_info,
                                 DWORD dest_context, void *dest_context_data,
-                                IRpcChannelBuffer **chan)
+                                IRpcChannelBuffer **chan, APARTMENT *apt)
 {
     ClientRpcChannelBuffer *This;
     WCHAR                   endpoint[200];
@@ -1148,7 +1152,7 @@ HRESULT RPC_CreateClientChannel(const OXID *oxid, const IPID *ipid,
     This->super.dest_context = dest_context;
     This->super.dest_context_data = dest_context_data;
     This->bind = bind;
-    apartment_getoxid(COM_CurrentApt(), &This->oxid);
+    apartment_getoxid(apt, &This->oxid);
     This->server_pid = oxid_info->dwPid;
     This->event = NULL;

diff --git a/dlls/ole32/tests/marshal.c b/dlls/ole32/tests/marshal.c
index 12c46e9..c5c69f0 100644
--- a/dlls/ole32/tests/marshal.c
+++ b/dlls/ole32/tests/marshal.c
@@ -3419,6 +3419,119 @@ static void test_manualresetevent(void)
     ok(!ref, "Got nonzero ref: %d\n", ref);
 }

+static DWORD CALLBACK implicit_mta_unmarshal_proc(void *param)
+{
+    IStream *stream = param;
+    IClassFactory *cf;
+    IUnknown *proxy;
+    HRESULT hr;
+
+    IStream_Seek(stream, ullZero, STREAM_SEEK_SET, NULL);
+    hr = CoUnmarshalInterface(stream, &IID_IClassFactory, (void **)&cf);
+    ok_ole_success(hr, CoUnmarshalInterface);
+
+    hr = IClassFactory_CreateInstance(cf, NULL, &IID_IUnknown, (void **)&proxy);
+    ok_ole_success(hr, IClassFactory_CreateInstance);
+
+    IUnknown_Release(proxy);
+
+    /* But if we initialize an STA in this apartment, it becomes the wrong one. */
+    CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
+
+    hr = IClassFactory_CreateInstance(cf, NULL, &IID_IUnknown, (void **)&proxy);
+    ok(hr == RPC_E_WRONG_THREAD, "got %#x\n", hr);
+
+    CoUninitialize();
+
+    ok_more_than_one_lock();
+    ok_non_zero_external_conn();
+
+    IClassFactory_Release(cf);
+
+    ok_no_locks();
+    ok_zero_external_conn();
+    ok_last_release_closes(TRUE);
+    return 0;
+}
+
+static DWORD CALLBACK implicit_mta_use_proc(void *param)
+{
+    IClassFactory *cf = param;
+    IUnknown *proxy;
+    HRESULT hr;
+
+    hr = IClassFactory_CreateInstance(cf, NULL, &IID_IUnknown, (void **)&proxy);
+    ok_ole_success(hr, IClassFactory_CreateInstance);
+
+    IUnknown_Release(proxy);
+
+    /* But if we initialize an STA in this apartment, it becomes the wrong one. */
+    CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
+
+    hr = IClassFactory_CreateInstance(cf, NULL, &IID_IUnknown, (void **)&proxy);
+    ok(hr == RPC_E_WRONG_THREAD, "got %#x\n", hr);
+
+    CoUninitialize();
+    return 0;
+}
+
+static void test_implicit_mta(void)
+{
+    HANDLE host_thread, thread;
+    IClassFactory *cf;
+    IStream *stream;
+    HRESULT hr;
+    DWORD tid;
+
+    cLocks = 0;
+    external_connections = 0;
+
+    CoInitializeEx(NULL, COINIT_MULTITHREADED);
+
+    /* Firstly: we can unmarshal and use an object while in the implicit MTA. */
+    hr = CreateStreamOnHGlobal(NULL, TRUE, &stream);
+    ok_ole_success(hr, CreateStreamOnHGlobal);
+    tid = start_host_object(stream, &IID_IClassFactory, (IUnknown *)&Test_ClassFactory, MSHLFLAGS_NORMAL, &host_thread);
+
+    ok_more_than_one_lock();
+    ok_non_zero_external_conn();
+
+    thread = CreateThread(NULL, 0, implicit_mta_unmarshal_proc, stream, 0, NULL);
+    ok(!WaitForSingleObject(thread, 1000), "wait failed\n");
+    CloseHandle(thread);
+
+    IStream_Release(stream);
+    end_host_object(tid, host_thread);
+
+    /* Secondly: we can unmarshal an object into the real MTA and then use it
+     * from the implicit MTA. */
+    hr = CreateStreamOnHGlobal(NULL, TRUE, &stream);
+    ok_ole_success(hr, CreateStreamOnHGlobal);
+    tid = start_host_object(stream, &IID_IClassFactory, (IUnknown *)&Test_ClassFactory, MSHLFLAGS_NORMAL, &host_thread);
+
+    ok_more_than_one_lock();
+    ok_non_zero_external_conn();
+
+    IStream_Seek(stream, ullZero, STREAM_SEEK_SET, NULL);
+    hr = CoUnmarshalInterface(stream, &IID_IClassFactory, (void **)&cf);
+    ok_ole_success(hr, CoUnmarshalInterface);
+
+    thread = CreateThread(NULL, 0, implicit_mta_use_proc, cf, 0, NULL);
+    ok(!WaitForSingleObject(thread, 1000), "wait failed\n");
+    CloseHandle(thread);
+
+    IClassFactory_Release(cf);
+    IStream_Release(stream);
+
+    ok_no_locks();
+    ok_non_zero_external_conn();
+    ok_last_release_closes(TRUE);
+
+    end_host_object(tid, host_thread);
+
+    CoUninitialize();
+}
+
 static const char *debugstr_iid(REFIID riid)
 {
     static char name[256];
@@ -3765,6 +3878,7 @@ START_TEST(marshal)
     register_test_window();

test_cocreateinstance_proxy();
+    test_implicit_mta();

pCoInitializeEx(NULL, COINIT_APARTMENTTHREADED);

[PATCH 6/6] ole32: Allow marshalling objects from an implicit MTA.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 dlls/ole32/compobj_private.h |  2 +-
 dlls/ole32/marshal.c         |  5 +++--
 dlls/ole32/rpc.c             |  2 +-
 dlls/ole32/stubmanager.c     |  3 +--
 dlls/ole32/tests/marshal.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/dlls/ole32/compobj_private.h b/dlls/ole32/compobj_private.h
index dc09d20..212d328 100644
--- a/dlls/ole32/compobj_private.h
+++ b/dlls/ole32/compobj_private.h
@@ -200,7 +200,7 @@ void stub_manager_release_marshal_data(struct stub_manager *m, ULONG refs, const
 void stub_manager_disconnect(struct stub_manager *m) DECLSPEC_HIDDEN;
 HRESULT ipid_get_dispatch_params(const IPID *ipid, APARTMENT **stub_apt, struct stub_manager **manager, IRpcStubBuffer **stub,
                                  IRpcChannelBuffer **chan, IID *iid, IUnknown **iface) DECLSPEC_HIDDEN;
-HRESULT start_apartment_remote_unknown(void) DECLSPEC_HIDDEN;
+HRESULT start_apartment_remote_unknown(APARTMENT *apt) DECLSPEC_HIDDEN;

HRESULT marshal_object(APARTMENT *apt, STDOBJREF *stdobjref, REFIID riid, IUnknown *obj, DWORD dest_context, void *dest_context_data, MSHLFLAGS mshlflags) DECLSPEC_HIDDEN;

diff --git a/dlls/ole32/marshal.c b/dlls/ole32/marshal.c
index 822d06e..7c0f541 100644
--- a/dlls/ole32/marshal.c
+++ b/dlls/ole32/marshal.c
@@ -1225,11 +1225,11 @@ StdMarshalImpl_MarshalInterface(
     STDOBJREF             stdobjref;
     ULONG                 res;
     HRESULT               hres;
-    APARTMENT            *apt = COM_CurrentApt();
+    APARTMENT *apt;

TRACE("(...,%s,...)\n", debugstr_guid(riid));

-    if (!apt)
+    if (!(apt = apartment_get_current_or_mta()))
     {
         ERR("Apartment not initialized\n");
         return CO_E_NOTINITIALIZED;
@@ -1239,6 +1239,7 @@ StdMarshalImpl_MarshalInterface(
     RPC_StartRemoting(apt);

hres = marshal_object(apt, &stdobjref, riid, pv, dest_context, dest_context_data, mshlflags);
+    apartment_release(apt);
     if (hres != S_OK)
     {
         ERR("Failed to create ifstub, hres=0x%x\n", hres);
diff --git a/dlls/ole32/rpc.c b/dlls/ole32/rpc.c
index 5a7626b..a73d23c 100644
--- a/dlls/ole32/rpc.c
+++ b/dlls/ole32/rpc.c
@@ -1648,7 +1648,7 @@ void RPC_StartRemoting(struct apartment *apt)

/* FIXME: move remote unknown exporting into this function */
     }
-    start_apartment_remote_unknown();
+    start_apartment_remote_unknown(apt);
 }

diff --git a/dlls/ole32/stubmanager.c b/dlls/ole32/stubmanager.c
index 57048c6..75c4b04 100644
--- a/dlls/ole32/stubmanager.c
+++ b/dlls/ole32/stubmanager.c
@@ -812,11 +812,10 @@ static const IRemUnknownVtbl RemUnknown_Vtbl =
 };

/* starts the IRemUnknown listener for the current apartment */
-HRESULT start_apartment_remote_unknown(void)
+HRESULT start_apartment_remote_unknown(APARTMENT *apt)
 {
     IRemUnknown *pRemUnknown;
     HRESULT hr = S_OK;
-    APARTMENT *apt = COM_CurrentApt();

EnterCriticalSection(&apt->cs);
     if (!apt->remunk_exported)
diff --git a/dlls/ole32/tests/marshal.c b/dlls/ole32/tests/marshal.c
index c5c69f0..e8558dd 100644
--- a/dlls/ole32/tests/marshal.c
+++ b/dlls/ole32/tests/marshal.c
@@ -3475,10 +3475,34 @@ static DWORD CALLBACK implicit_mta_use_proc(void *param)
     return 0;
 }

+struct implicit_mta_marshal_data
+{
+    IStream *stream;
+    HANDLE start;
+    HANDLE stop;
+};
+
+static DWORD CALLBACK implicit_mta_marshal_proc(void *param)
+{
+    struct implicit_mta_marshal_data *data = param;
+    HRESULT hr;
+
+    hr = CoMarshalInterface(data->stream, &IID_IClassFactory,
+        (IUnknown *)&Test_ClassFactory, MSHCTX_INPROC, NULL, MSHLFLAGS_NORMAL);
+    ok_ole_success(hr, CoMarshalInterface);
+
+    SetEvent(data->start);
+
+    ok(!WaitForSingleObject(data->stop, 1000), "wait failed\n");
+    return 0;
+}
+
 static void test_implicit_mta(void)
 {
+    struct implicit_mta_marshal_data data;
     HANDLE host_thread, thread;
     IClassFactory *cf;
+    IUnknown *proxy;
     IStream *stream;
     HRESULT hr;
     DWORD tid;
@@ -3529,6 +3553,32 @@ static void test_implicit_mta(void)

end_host_object(tid, host_thread);

+    /* Thirdly: we can marshal an object from the implicit MTA and then
+     * unmarshal it into the real one. */
+    data.start = CreateEventA(NULL, FALSE, FALSE, NULL);
+    data.stop  = CreateEventA(NULL, FALSE, FALSE, NULL);
+
+    hr = CreateStreamOnHGlobal(NULL, TRUE, &data.stream);
+    ok_ole_success(hr, CreateStreamOnHGlobal);
+
+    thread = CreateThread(NULL, 0, implicit_mta_marshal_proc, &data, 0, NULL);
+    ok(!WaitForSingleObject(data.start, 1000), "wait failed\n");
+
+    IStream_Seek(data.stream, ullZero, STREAM_SEEK_SET, NULL);
+    hr = CoUnmarshalInterface(data.stream, &IID_IClassFactory, (void **)&cf);
+    ok_ole_success(hr, CoUnmarshalInterface);
+
+    hr = IClassFactory_CreateInstance(cf, NULL, &IID_IUnknown, (void **)&proxy);
+    ok_ole_success(hr, IClassFactory_CreateInstance);
+
+    IUnknown_Release(proxy);
+
+    SetEvent(data.stop);
+    ok(!WaitForSingleObject(thread, 1000), "wait failed\n");
+    CloseHandle(thread);
+
+    IStream_Release(data.stream);
+
     CoUninitialize();
 }

[Huw Davies]

On Sun, Mar 25, 2018 at 12:33:57PM -0500, Zebediah Figura wrote:
> Signed-off-by: Zebediah Figura
>
> dlls/ole32/tests/compobj.c | 229 ++++++++++-----------------------------------
>  1 file changed, 47 insertions(+), 182 deletions(-)
>
> diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c
> index f1e0134..da4223b 100644
> --- a/dlls/ole32/tests/compobj.c
> +++ b/dlls/ole32/tests/compobj.c
> +static DWORD CALLBACK implicit_mta_proc(void *param)
> +{
> - IComThreadingInfo *threading_info;
> - ULONG_PTR token;
> - IUnknown *unk;
> - CLSID clsid;

clsid is unused at this point.  I know you'll use it later on.  If I wasn't
going to ask for other changes, I'd ignore it.
> - 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;
>
> +}

[Zebediah Figura]

On 28/03/18 03:26, Huw Davies wrote:
> On Sun, Mar 25, 2018 at 12:33:57PM -0500, Zebediah Figura wrote:
> > Signed-off-by: Zebediah Figura
> >
> > dlls/ole32/tests/compobj.c | 229 ++++++++++-----------------------------------
> >  1 file changed, 47 insertions(+), 182 deletions(-)
> >
> > diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c
> > index f1e0134..da4223b 100644
> > --- a/dlls/ole32/tests/compobj.c
> > +++ b/dlls/ole32/tests/compobj.c
> > +static DWORD CALLBACK implicit_mta_proc(void *param)
> > +{
> > - IComThreadingInfo *threading_info;
> > - ULONG_PTR token;
> > - IUnknown *unk;
> > - CLSID clsid;
>
> clsid is unused at this point.  I know you'll use it later on.  If I wasn't
> going to ask for other changes, I'd ignore it.

Ah, whoops, I meant to fix that :-/ Thanks for pointing it out.