On Mon, Mar 14, 2016 at 08:10:00AM +0300, Nikolay Sivov wrote:
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
dlls/ole32/compobj.c | 57 +++++++++++++++++++--------------------------- dlls/ole32/tests/compobj.c | 46 ++++++++++++++++++++++--------------- 2 files changed, 51 insertions(+), 52 deletions(-)
diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c index b095aeb..1081f98 100644 --- a/dlls/ole32/compobj.c +++ b/dlls/ole32/compobj.c @@ -4683,10 +4683,13 @@ static ULONG Context_AddRef(Context *This)
static ULONG Context_Release(Context *This) {
- ULONG refs = InterlockedDecrement(&This->refs);
- if (!refs)
- if (!This->refs)
- {
Since this is non-standard it needs a comment. Probably just referring to the constructor below.
HeapFree(GetProcessHeap(), 0, This);
- return refs;
return 0;- }
- return InterlockedDecrement(&This->refs);
}
static HRESULT WINAPI Context_CTI_QueryInterface(IComThreadingInfo *iface, REFIID riid, LPVOID *ppv) @@ -4924,39 +4927,19 @@ static const IObjContextVtbl Context_Object_Vtbl = */ HRESULT WINAPI CoGetObjectContext(REFIID riid, void **ppv) {
- APARTMENT *apt = COM_CurrentApt();
- Context *context;
IObjContext *context; HRESULT hr;
TRACE("(%s, %p)\n", debugstr_guid(riid), ppv);
*ppv = NULL;
- if (!apt)
- {
if (!(apt = apartment_find_multi_threaded())){ERR("apartment not initialised\n");return CO_E_NOTINITIALIZED;}apartment_release(apt);- }
- context = HeapAlloc(GetProcessHeap(), 0, sizeof(*context));
- if (!context)
return E_OUTOFMEMORY;- context->IComThreadingInfo_iface.lpVtbl = &Context_Threading_Vtbl;
- context->IContextCallback_iface.lpVtbl = &Context_Callback_Vtbl;
- context->IObjContext_iface.lpVtbl = &Context_Object_Vtbl;
- context->refs = 1;
- hr = IComThreadingInfo_QueryInterface(&context->IComThreadingInfo_iface, riid, ppv);
- IComThreadingInfo_Release(&context->IComThreadingInfo_iface);
- hr = CoGetContextToken((ULONG_PTR*)&context);
- if (FAILED(hr))
return hr;
- return hr;
- return IObjContext_QueryInterface(context, riid, ppv);
}
/***********************************************************************
CoGetContextToken [OLE32.@]*/ @@ -4985,16 +4968,22 @@ HRESULT WINAPI CoGetContextToken( ULONG_PTR *token )
if (!info->context_token) {
HRESULT hr;IObjContext *ctx;
Context *context;context = HeapAlloc(GetProcessHeap(), 0, sizeof(*context));if (!context)return E_OUTOFMEMORY;context->IComThreadingInfo_iface.lpVtbl = &Context_Threading_Vtbl;context->IContextCallback_iface.lpVtbl = &Context_Callback_Vtbl;context->IObjContext_iface.lpVtbl = &Context_Object_Vtbl;
Add a comment here stating that context_token does not take a ref.
context->refs = 0;
hr = CoGetObjectContext(&IID_IObjContext, (void **)&ctx);if (FAILED(hr)) return hr;info->context_token = ctx;
info->context_token = &context->IObjContext_iface;}
*token = (ULONG_PTR)info->context_token;
- TRACE("apt->context_token=%p\n", info->context_token);
TRACE("context_token=%p\n", info->context_token);
return S_OK;
}
I'm wondering whether this patch could be split since it's doing rather more than fixing a ref counting issue. Could you move the constructor to CoGetContextToken first and then fix the refs, or would that mess up the tests?
Huw.