[PATCH 5/6] ole32: Allow unmarshalling objects into an implicit MTA.
Zebediah Figura
z.figura12 at gmail.com
Sun Mar 25 12:34:01 CDT 2018
Signed-off-by: Zebediah Figura <z.figura12 at gmail.com>
---
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);
--
2.7.4
More information about the wine-devel
mailing list