Re: [PATCH v2] ole32: better match native behaviour in OLE init/uninit ole32/tests: check COM/OLE init state after mixed COM/OLE init/uninit

[Huw Davies]

On Wed, Jan 13, 2016 at 01:16:42PM +0300, Paul Gofman wrote:
> Native OleUnitialize never uninits apartment inited by CoInitializeEx.
> CoUnitialize after OleInitialize uninits apartment, but Ole is still considered
> inited.
> Signed-off-by: Paul Gofman
>
> dlls/ole32/compobj.c       |  2 ++
>  dlls/ole32/ole2.c          |  7 +++++
>  dlls/ole32/tests/compobj.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 79 insertions(+)
>
> diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
> index 90b049a..78fc30d 100644
> --- a/dlls/ole32/compobj.c
> +++ b/dlls/ole32/compobj.c
> @@ -1972,6 +1972,8 @@ void WINAPI DECLSPEC_HOTPATCH CoUninitialize(void)
>
> if (!--info->inits)
>    {
> - if (info->ole_inits)
> -
>  ERR("uninitializing apartment while Ole is still initialized\n");

This should be a WARN since it's likely to be an application, not a
Wine, error.
>  apartment_release(info->apt);
>  info->apt = NULL;
>
> }
> diff --git a/dlls/ole32/ole2.c b/dlls/ole32/ole2.c
> index 7929d3d..f078fb6 100644
> --- a/dlls/ole32/ole2.c
> +++ b/dlls/ole32/ole2.c
> @@ -186,6 +186,8 @@ HRESULT WINAPI DECLSPEC_HOTPATCH OleInitialize(LPVOID reserved)
>
> if (!COM_CurrentInfo()->ole_inits)
>      hr = S_OK;
> -
>
> else
> -
>
> hr = S_FALSE;
>
> /*
> - Then, it has to initialize the OLE specific modules.
>
> @@ -229,6 +231,11 @@ void WINAPI DECLSPEC_HOTPATCH OleUninitialize(void)
>  {
>    TRACE("()\n");
> - if (COM_CurrentInfo()->ole_inits == 0)
> - {
> - WARN("ole_inits is already 0\n");
> - return ;
> - }
> /*
> - If we hit the bottom of the lock stack, free the libraries.
> */
>
> diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c
> index 5fe9876..3ee5470 100644
> --- a/dlls/ole32/tests/compobj.c
> +++ b/dlls/ole32/tests/compobj.c
> @@ -2181,6 +2181,75 @@ static void test_CoInitializeEx(void)
>      OleUninitialize();
>  }
>
> +static void test_OleInitialize_InitCounting(void)
> +{
> - HRESULT hr;
> - HANDLE thread;
> - IUnknown *pUnk;
> - REFCLSID rclsid = &CLSID_InternetZoneManager;
> - IDataObject *pDObj;

compobj.c: In function ‘test_OleInitialize_InitCounting’:
compobj.c:2190: warning: unused variable ‘pDObj’
compobj.c:2187: warning: unused variable ‘thread’

[PATCH v2] ole32: better match native behaviour in OLE init/uninit ole32/tests: check COM/OLE init state after mixed COM/OLE init/uninit

[Paul Gofman]

On 01/15/2016 01:08 PM, Huw Davies wrote:
> On Wed, Jan 13, 2016 at 01:16:42PM +0300, Paul Gofman wrote:
> > Native OleUnitialize never uninits apartment inited by CoInitializeEx.
> > CoUnitialize after OleInitialize uninits apartment, but Ole is still considered
> > inited.
> > Signed-off-by: Paul Gofman
> >
> > dlls/ole32/compobj.c       |  2 ++
> >  dlls/ole32/ole2.c          |  7 +++++
> >  dlls/ole32/tests/compobj.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 79 insertions(+)
> >
> > diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
> > index 90b049a..78fc30d 100644
> > --- a/dlls/ole32/compobj.c
> > +++ b/dlls/ole32/compobj.c
> > @@ -1972,6 +1972,8 @@ void WINAPI DECLSPEC_HOTPATCH CoUninitialize(void)
> >
> > if (!--info->inits)
> >    {
> > - if (info->ole_inits)
> > -
> >  ERR("uninitializing apartment while Ole is still initialized\n");
>
> This should be a WARN since it's likely to be an application, not a
> Wine, error.

I intentionally put an ERR here as I thought that most likely the
behaviour in this case will be different from native. I also thought it
would be easier to catch the source of the problem if this case will be
encountered in the real application. I will change it to WARN now.

Thanks,
    Paul.