Re: howto fix bug 13462

[Louis. Lenders]

Rob Shearman <robertshearman <at> gmail.com> writes:

Hi Rob, i think i got the test ready now, i'll send it when the defenitions are
in place in ctxtcall.idl. Now still strugling how to fix the bug.

 I fixed up the patch below, also by looking how things are done in similar
places in wine-code. With patch applied I can see that
ContextCallback_ContextCallback and ContextCallback_Release are called from the
application (tested with NASA WorldWind), and the simple stubs are enough to let
the app continue fine. But i'm not sure if everything is 100% ok. Is this what
you more or less meant in your first comment?

diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
index 37907ec..a1f41fd 100644
--- a/dlls/ole32/compobj.c
+++ b/dlls/ole32/compobj.c
@@ -55,6 +55,7 @@
 #include "objbase.h"
 #include "ole2.h"
 #include "ole2ver.h"
+#include "ctxtcall.h"
 
 #include "compobj_private.h"
 
@@ -3615,25 +3616,39 @@ HRESULT WINAPI CoRegisterChannelHook(REFGUID guidExtension, IChannelHook *pChann
 typedef struct Context
 {
     const IComThreadingInfoVtbl *lpVtbl;
+    const IContextCallback *lpVtblIContextCallback;
     LONG refs;
     APTTYPE apttype;
 } Context;
 
+static inline Context *impl_from_Context(IComThreadingInfo *iface)
+{
+    return (Context *)((char*)iface - FIELD_OFFSET(Context, lpVtbl));
+}
+
 static HRESULT WINAPI Context_QueryInterface(IComThreadingInfo *iface, REFIID riid, LPVOID *ppv)
 {
+    Context* This=impl_from_Context((IComThreadingInfo*) iface);
     *ppv = NULL;
 
     if (IsEqualIID(riid, &IID_IComThreadingInfo) ||
         IsEqualIID(riid, &IID_IUnknown))
     {
         *ppv = iface;
-        IUnknown_AddRef(iface);
-        return S_OK;
     }
+    else if(IsEqualIID(riid, &IID_IContextCallback))
+        {
+        *ppv = &This->lpVtblIContextCallback;
+        }
+    else
+        {
+        FIXME("interface not implemented %s\n", debugstr_guid(riid));
+        return E_NOINTERFACE;
+        }
 
-    FIXME("interface not implemented %s\n", debugstr_guid(riid));
-    return E_NOINTERFACE;
-}
+    IUnknown_AddRef(iface);
+    return S_OK;
+}    
 
 static ULONG WINAPI Context_AddRef(IComThreadingInfo *iface)
 {
@@ -3691,6 +3706,31 @@ static HRESULT WINAPI Context_SetCurrentLogicalThreadId(IComThreadingInfo *iface
     return E_NOTIMPL;
 }
 
+
+static HRESULT WINAPI  ContextCallback_QueryInterface(IContextCallback *iface, REFIID riid, LPVOID *ppv)
+{
+    FIXME("\n");
+    return E_NOTIMPL;
+}
+
+static ULONG WINAPI  ContextCallback_AddRef(IContextCallback *iface)
+{
+    FIXME("\n");
+    return E_NOTIMPL;
+}
+
+static ULONG WINAPI ContextCallback_Release(IContextCallback *iface)
+{
+    FIXME("\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI ContextCallback_ContextCallback(IContextCallback *iface, PFNCONTEXTCALL pfnCallback, ComCallData* pParam, REFIID riid, int iMethod, IUnknown* pUnk)
+{
+    FIXME("not implemented yet\n");
+    return E_NOTIMPL;
+}
+
 static const IComThreadingInfoVtbl Context_Threading_Vtbl =
 {
     Context_QueryInterface,
@@ -3702,6 +3742,14 @@ static const IComThreadingInfoVtbl Context_Threading_Vtbl =
     Context_SetCurrentLogicalThreadId
 };
 
+static const IContextCallbackVtbl Context_Callback_Vtbl =
+{
+    ContextCallback_QueryInterface,
+    ContextCallback_AddRef,
+    ContextCallback_Release,
+    ContextCallback_ContextCallback
+};
+
 /***********************************************************************
  *           CoGetObjectContext [OLE32.@]
  *
@@ -3735,6 +3783,7 @@ HRESULT WINAPI CoGetObjectContext(REFIID riid, void **ppv)
         return E_OUTOFMEMORY;
 
     context->lpVtbl = &Context_Threading_Vtbl;
+    context->lpVtblIContextCallback=(IContextCallback*)&Context_Callback_Vtbl;
     context->refs = 1;
     if (apt->multi_threaded)
         context->apttype = APTTYPE_MTA;

howto fix bug 13462

[Rob Shearman]

2008/9/29 Louis. Lenders <xerox_xerox2000(a)yahoo.co.uk>:
> Rob Shearman <robertshearman <at> gmail.com> writes:
>
> Hi Rob, i think i got the test ready now, i'll send it when the defenitions
> are
> in place in ctxtcall.idl. Now still strugling how to fix the bug.
>
> I fixed up the patch below, also by looking how things are done in similar
> places in wine-code. With patch applied I can see that
> ContextCallback_ContextCallback and ContextCallback_Release are called from
> the
> application (tested with NASA WorldWind), and the simple stubs are enough to
> let
> the app continue fine. But i'm not sure if everything is 100% ok. Is this
> what
> you more or less meant in your first comment?

Looks good apart from some small issues below:
> diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
> index 37907ec..a1f41fd 100644
> --- a/dlls/ole32/compobj.c
> +++ b/dlls/ole32/compobj.c
> @@ -55,6 +55,7 @@
>  #include "objbase.h"
>  #include "ole2.h"
>  #include "ole2ver.h"
> +#include "ctxtcall.h"
>
> #include "compobj_private.h"
>
> @@ -3615,25 +3616,39 @@ HRESULT WINAPI CoRegisterChannelHook(REFGUID
> guidExtension, IChannelHook *pChann
>  typedef struct Context
>  {
>      const IComThreadingInfoVtbl *lpVtbl;
> +    const IContextCallback *lpVtblIContextCallback;

This should be of type IContextCallbackVtbl. Also
s/lpVtblIContextCallback/lpVtblContextCallback/.
> LONG refs;
>      APTTYPE apttype;
>  } Context;
>
> +static inline Context *impl_from_Context(IComThreadingInfo *iface)
> +{
> +    return (Context *)((char*)iface - FIELD_OFFSET(Context, lpVtbl));
> +}
> +
>  static HRESULT WINAPI Context_QueryInterface(IComThreadingInfo *iface,
> REFIID riid, LPVOID *ppv)
>  {
> +    Context* This=impl_from_Context((IComThreadingInfo*) iface);
>      *ppv = NULL;
>
> if (IsEqualIID(riid, &IID_IComThreadingInfo) ||
>          IsEqualIID(riid, &IID_IUnknown))
>      {
>          *ppv = iface;
> -        IUnknown_AddRef(iface);
> -        return S_OK;
>      }
> +    else if(IsEqualIID(riid, &IID_IContextCallback))
> +        {
> +        *ppv = &This->lpVtblIContextCallback;
> +        }
> +    else
> +        {
> +        FIXME("interface not implemented %s\n", debugstr_guid(riid));
> +        return E_NOINTERFACE;
> +        }
>
> -    FIXME("interface not implemented %s\n", debugstr_guid(riid));
> -    return E_NOINTERFACE;
> -}
> +    IUnknown_AddRef(iface);
> +    return S_OK;
> +}
>
> static ULONG WINAPI Context_AddRef(IComThreadingInfo *iface)
>  {
> @@ -3691,6 +3706,31 @@ static HRESULT WINAPI
> Context_SetCurrentLogicalThreadId(IComThreadingInfo *iface
>      return E_NOTIMPL;
>  }
>
> +
> +static HRESULT WINAPI  ContextCallback_QueryInterface(IContextCallback
> *iface, REFIID riid, LPVOID *ppv)
> +{
> +    FIXME("\n");
> +    return E_NOTIMPL;
> +}
> +
> +static ULONG WINAPI  ContextCallback_AddRef(IContextCallback *iface)
> +{
> +    FIXME("\n");
> +    return E_NOTIMPL;
> +}
> +
> +static ULONG WINAPI ContextCallback_Release(IContextCallback *iface)
> +{
> +    FIXME("\n");
> +    return E_NOTIMPL;
> +}

These functions should be called Context_IContextCallback_* instead of
ContextCallback_* to show that they are part of the same object as the
Context_* functions. Also, you should be able to easily implement
these using impl_from_Context(IComThreadingInfo *iface) and
Context_QueryInterface, Context_AddRef and Context_Release.
> +
> +static HRESULT WINAPI ContextCallback_ContextCallback(IContextCallback
> *iface, PFNCONTEXTCALL pfnCallback, ComCallData* pParam, REFIID riid, int
> iMethod, IUnknown* pUnk)
> +{
> +    FIXME("not implemented yet\n");
> +    return E_NOTIMPL;
> +}

You should dump the parameters in the FIXME so that we have more of an
idea of what is going on. Something like "(%p, %p, %s, %d, %p):
stub\n" would be appropriate.
> +
>  static const IComThreadingInfoVtbl Context_Threading_Vtbl =
>  {
>      Context_QueryInterface,
> @@ -3702,6 +3742,14 @@ static const IComThreadingInfoVtbl
> Context_Threading_Vtbl =
>      Context_SetCurrentLogicalThreadId
>  };
>
> +static const IContextCallbackVtbl Context_Callback_Vtbl =
> +{
> +    ContextCallback_QueryInterface,
> +    ContextCallback_AddRef,
> +    ContextCallback_Release,
> +    ContextCallback_ContextCallback
> +};
> +
>  /***********************************************************************
>   *           CoGetObjectContext [OLE32.@]
>   *
> @@ -3735,6 +3783,7 @@ HRESULT WINAPI CoGetObjectContext(REFIID riid, void
> **ppv)
>          return E_OUTOFMEMORY;
>
> context->lpVtbl = &Context_Threading_Vtbl;
> +
> context->lpVtblIContextCallback=(IContextCallback*)&Context_Callback_Vtbl;

You can remove this typecast when the type of lpVtblContextCallback is fixed.
> context->refs = 1;
>      if (apt->multi_threaded)
>          context->apttype = APTTYPE_MTA;

-- 
Rob Shearman

howto fix bug 13462

[Louis Lenders]

Rob Shearman <robertshearman <at> gmail.com> writes:
Thanks very much for your comments Rob,
I've updated the patch here:
http://bugs.winehq.org/attachment.cgi?id=16397

Hopefully correct now?

But now got another problem with the test, i changed it a bit like here:
http://bugs.winehq.org/attachment.cgi?id=16430

This test passes fine in Xp, but not in wine, more specific, this part fails:
pCoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
.
.
.
.     if(hr == S_OK) 
        refs = IContextCallback_Release(pComThreadingInfo);
    
    ok(refs == 0, "pComThreadingInfo should have 0 refs instead of %d refs\n",
refs);
 
In windows it shows the refcount should be zero in the end, but with my patch
and this test, it doesn't (refcount =2 in the end). I added several traces to
the code to show how the refcount was in/decreased, but wasn't able to figure
out what goes wrong, it's quite mysterious. Whereas this test fails, the one
that starts with 
pCoInitializeEx(NULL, COINIT_MULTITHREADED) passes just fine in wine. Am i still
missing something?