[PATCH v2] quartz: Change reference increasing method to avoid unintended DLL unloading.

[Hiroki Awata]

Incrementing object_locks by DSCF_CreateInstance is sometimes wrong. For example, dsound_render_create creates two instances because it calls system_clock_create internally, but object_locks is only increased by 1.
Each create function should increase the object_locks instead of DSCF_CreateInstance.

Signed-off-by: Hiroki Awata 
---
 dlls/quartz/acmwrapper.c   | 1 +
 dlls/quartz/avidec.c       | 1 +
 dlls/quartz/dsoundrender.c | 1 +
 dlls/quartz/filesource.c   | 1 +
 dlls/quartz/filtergraph.c  | 1 +
 dlls/quartz/filtermapper.c | 3 +++
 dlls/quartz/main.c         | 1 -
 dlls/quartz/memallocator.c | 1 +
 dlls/quartz/passthrough.c  | 1 +
 dlls/quartz/systemclock.c  | 1 +
 dlls/quartz/vmr9.c         | 1 +
 11 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/dlls/quartz/acmwrapper.c b/dlls/quartz/acmwrapper.c
index 81e856a00df..0804c18b3c8 100644
--- a/dlls/quartz/acmwrapper.c
+++ b/dlls/quartz/acmwrapper.c
@@ -534,6 +534,7 @@ HRESULT acm_wrapper_create(IUnknown *outer, IUnknown **out)

object->lasttime_real = object->lasttime_sent = -1;

+    InterlockedIncrement(&object_locks);
     TRACE("Created ACM wrapper %p.\n", object);
     *out = &object->filter.IUnknown_inner;

diff --git a/dlls/quartz/avidec.c b/dlls/quartz/avidec.c
index b1e07fe5879..8191b703553 100644
--- a/dlls/quartz/avidec.c
+++ b/dlls/quartz/avidec.c
@@ -611,6 +611,7 @@ HRESULT avi_dec_create(IUnknown *outer, IUnknown **out)
     ISeekingPassThru_Init(&object->passthrough.ISeekingPassThru_iface, FALSE,
             &object->sink.pin.IPin_iface);

+    InterlockedIncrement(&object_locks);
     TRACE("Created AVI decompressor %p.\n", object);
     *out = &object->filter.IUnknown_inner;

diff --git a/dlls/quartz/dsoundrender.c b/dlls/quartz/dsoundrender.c
index 68c88ea5a15..521a97ca359 100644
--- a/dlls/quartz/dsoundrender.c
+++ b/dlls/quartz/dsoundrender.c
@@ -1049,6 +1049,7 @@ HRESULT dsound_render_create(IUnknown *outer, IUnknown **out)
     object->IAMDirectSound_iface.lpVtbl = &IAMDirectSound_Vtbl;
     object->IQualityControl_iface.lpVtbl = &dsound_render_qc_vtbl;

+    InterlockedIncrement(&object_locks);
     TRACE("Created DirectSound renderer %p.\n", object);
     *out = &object->filter.IUnknown_inner;

diff --git a/dlls/quartz/filesource.c b/dlls/quartz/filesource.c
index cf5fe457715..12830d404b8 100644
--- a/dlls/quartz/filesource.c
+++ b/dlls/quartz/filesource.c
@@ -430,6 +430,7 @@ HRESULT async_reader_create(IUnknown *outer, IUnknown **out)
     object->port = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0);
     object->io_thread = CreateThread(NULL, 0, io_thread, object, 0, NULL);

+    InterlockedIncrement(&object_locks);
     TRACE("Created file source %p.\n", object);
     *out = &object->filter.IUnknown_inner;
     return S_OK;
diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c
index a1f3757a202..6b13fca6787 100644
--- a/dlls/quartz/filtergraph.c
+++ b/dlls/quartz/filtergraph.c
@@ -5652,6 +5652,7 @@ static HRESULT filter_graph_common_create(IUnknown *outer, IUnknown **out, BOOL
     else
         object->message_thread = NULL;

+    InterlockedIncrement(&object_locks);
     TRACE("Created %sthreaded filter graph %p.\n", threaded ? "" : "non-", object);
     *out = &object->IUnknown_inner;
     return S_OK;
diff --git a/dlls/quartz/filtermapper.c b/dlls/quartz/filtermapper.c
index b3210fdcd59..249867da8c8 100644
--- a/dlls/quartz/filtermapper.c
+++ b/dlls/quartz/filtermapper.c
@@ -195,6 +195,7 @@ static HRESULT enum_reg_filters_create(REGFILTER *filters, unsigned int count, I
     object->refcount = 1;
     object->count = count;

+    InterlockedIncrement(&object_locks);
     TRACE("Created enumerator %p.\n", object);
     *out = &object->IEnumRegFilters_iface;
     return S_OK;
@@ -329,6 +330,7 @@ static HRESULT enum_moniker_create(IMoniker **filters, unsigned int count, IEnum
     object->refcount = 1;
     object->count = count;

+    InterlockedIncrement(&object_locks);
     TRACE("Created enumerator %p.\n", object);
     *out = &object->IEnumMoniker_iface;
     return S_OK;
@@ -1701,6 +1703,7 @@ HRESULT filter_mapper_create(IUnknown *pUnkOuter, IUnknown **out)
     else
         pFM2impl->outer_unk = &pFM2impl->IUnknown_inner;

+    InterlockedIncrement(&object_locks);
     TRACE("Created filter mapper %p.\n", pFM2impl);
     *out = &pFM2impl->IUnknown_inner;
     return S_OK;
diff --git a/dlls/quartz/main.c b/dlls/quartz/main.c
index 303cc30a931..282c56e6c7f 100644
--- a/dlls/quartz/main.c
+++ b/dlls/quartz/main.c
@@ -127,7 +127,6 @@ static HRESULT WINAPI DSCF_CreateInstance(IClassFactory *iface, IUnknown *pOuter

if (SUCCEEDED(hres = This->create_instance(pOuter, &punk)))
     {
-        InterlockedIncrement(&object_locks);
         hres = IUnknown_QueryInterface(punk, riid, ppobj);
         IUnknown_Release(punk);
     }
diff --git a/dlls/quartz/memallocator.c b/dlls/quartz/memallocator.c
index cc13f776cfb..1bc4d1d0041 100644
--- a/dlls/quartz/memallocator.c
+++ b/dlls/quartz/memallocator.c
@@ -936,5 +936,6 @@ HRESULT mem_allocator_create(IUnknown *lpUnkOuter, IUnknown **out)
     else
         CoTaskMemFree(pMemAlloc);

+    InterlockedIncrement(&object_locks);
     return hr;
 }
diff --git a/dlls/quartz/passthrough.c b/dlls/quartz/passthrough.c
index 7f81ae8a1be..e576547f934 100644
--- a/dlls/quartz/passthrough.c
+++ b/dlls/quartz/passthrough.c
@@ -108,6 +108,7 @@ HRESULT seeking_passthrough_create(IUnknown *outer, IUnknown **out)

strmbase_passthrough_init(&object->passthrough, object->outer_unk);

+    InterlockedIncrement(&object_locks);
     TRACE("Created seeking passthrough %p.\n", object);
     *out = &object->IUnknown_inner;
     return S_OK;
diff --git a/dlls/quartz/systemclock.c b/dlls/quartz/systemclock.c
index 73a923a7b99..917cde99681 100644
--- a/dlls/quartz/systemclock.c
+++ b/dlls/quartz/systemclock.c
@@ -339,6 +339,7 @@ HRESULT system_clock_create(IUnknown *outer, IUnknown **out)
     InitializeCriticalSection(&object->cs);
     object->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": SystemClockImpl.cs");

+    InterlockedIncrement(&object_locks);
     TRACE("Created system clock %p.\n", object);
     *out = &object->IUnknown_inner;

diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c
index 2562b74932a..df50a32d514 100644
--- a/dlls/quartz/vmr9.c
+++ b/dlls/quartz/vmr9.c
@@ -2587,6 +2587,7 @@ static HRESULT vmr_create(IUnknown *outer, IUnknown **out, const CLSID *clsid)
     object->mixing_prefs = MixerPref9_NoDecimation | MixerPref9_ARAdjustXorY
             | MixerPref9_BiLinearFiltering | MixerPref9_RenderTargetRGB;

+    InterlockedIncrement(&object_locks);
     TRACE("Created VMR %p.\n", object);
     *out = &object->renderer.filter.IUnknown_inner;
     return S_OK;

[Zebediah Figura (she/her)]

On 8/19/21 5:11 AM, Hiroki Awata wrote:
> diff --git a/dlls/quartz/filtermapper.c b/dlls/quartz/filtermapper.c
> index b3210fdcd59..249867da8c8 100644
> --- a/dlls/quartz/filtermapper.c
> +++ b/dlls/quartz/filtermapper.c
> @@ -195,6 +195,7 @@ static HRESULT enum_reg_filters_create(REGFILTER *filters, unsigned int count, I
>       object->refcount = 1;
>       object->count = count;
> - InterlockedIncrement(&object_locks);
>  TRACE("Created enumerator %p.\n", object);
>  *out = &object->IEnumRegFilters_iface;
>  return S_OK;
>
> @@ -329,6 +330,7 @@ static HRESULT enum_moniker_create(IMoniker **filters, unsigned int count, IEnum
>       object->refcount = 1;
>       object->count = count;
> - InterlockedIncrement(&object_locks);
>  TRACE("Created enumerator %p.\n", object);
>  *out = &object->IEnumMoniker_iface;
>  return S_OK;

These two are desirable, but it needs a corresponding decrement in the 
Release() methods. It should probably also be a separate patch.
> diff --git a/dlls/quartz/memallocator.c b/dlls/quartz/memallocator.c
> index cc13f776cfb..1bc4d1d0041 100644
> --- a/dlls/quartz/memallocator.c
> +++ b/dlls/quartz/memallocator.c
> @@ -936,5 +936,6 @@ HRESULT mem_allocator_create(IUnknown *lpUnkOuter, IUnknown **out)
>       else
>           CoTaskMemFree(pMemAlloc);
> - InterlockedIncrement(&object_locks);
>  return hr;
> }

This increases object_locks in the failure case.

video_renderer_create() also needs an increment; it seems to have been 
overlooked by this patch.

[Alexandre Julliard]

Hiroki Awata  writes:
> Incrementing object_locks by DSCF_CreateInstance is sometimes
> wrong. For example, dsound_render_create creates two instances because
> it calls system_clock_create internally, but object_locks is only
> increased by 1.  Each create function should increase the object_locks
> instead of DSCF_CreateInstance.

In general getting the lock count right is tricky and error-prone, and
not very useful. You can simply return FALSE from DllCanUnloadNow().