[PATCH v2] quartz: Change reference increasing method to avoid unintended DLL unloading.
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 <castaneai(a)by.black> --- 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; -- 2.27.0
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.
Hiroki Awata <castaneai(a)by.black> 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(). -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Hiroki Awata -
Zebediah Figura (she/her)