[PATCH v2 1/3] mmdevapi: Make MMDevEnumerator a static variable.

[Rémi Bernon]

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/mmdevapi/devenum.c | 40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c
index fc33cac17ec..a5863af2705 100644
--- a/dlls/mmdevapi/devenum.c
+++ b/dlls/mmdevapi/devenum.c
@@ -56,10 +56,9 @@ typedef struct MMDevPropStoreImpl
 typedef struct MMDevEnumImpl
 {
     IMMDeviceEnumerator IMMDeviceEnumerator_iface;
-    LONG ref;
+    BOOL initialized;
 } MMDevEnumImpl;
 
-static MMDevEnumImpl *MMDevEnumerator;
 static MMDevice **MMDevice_head;
 static MMDevice *MMDevice_def_rec, *MMDevice_def_play;
 static DWORD MMDevice_count;
@@ -69,6 +68,7 @@ static const IMMDeviceVtbl MMDeviceVtbl;
 static const IPropertyStoreVtbl MMDevPropVtbl;
 static const IMMEndpointVtbl MMEndpointVtbl;
 
+static MMDevEnumImpl enumerator;
 static IMMDevice info_device;
 
 typedef struct MMDevColImpl
@@ -860,23 +860,15 @@ static const IMMDeviceCollectionVtbl MMDevColVtbl =
 
 HRESULT MMDevEnum_Create(REFIID riid, void **ppv)
 {
-    MMDevEnumImpl *This = MMDevEnumerator;
-
-    if (!This)
+    if (!enumerator.initialized)
     {
-        This = HeapAlloc(GetProcessHeap(), 0, sizeof(*This));
-        *ppv = NULL;
-        if (!This)
-            return E_OUTOFMEMORY;
-        This->ref = 1;
-        This->IMMDeviceEnumerator_iface.lpVtbl = &MMDevEnumVtbl;
-        MMDevEnumerator = This;
-
+        enumerator.initialized = TRUE;
         load_devices_from_reg();
         load_driver_devices(eRender);
         load_driver_devices(eCapture);
     }
-    return IMMDeviceEnumerator_QueryInterface(&This->IMMDeviceEnumerator_iface, riid, ppv);
+
+    return IMMDeviceEnumerator_QueryInterface(&enumerator.IMMDeviceEnumerator_iface, riid, ppv);
 }
 
 void MMDevEnum_Free(void)
@@ -886,8 +878,6 @@ void MMDevEnum_Free(void)
     RegCloseKey(key_render);
     RegCloseKey(key_capture);
     key_render = key_capture = NULL;
-    HeapFree(GetProcessHeap(), 0, MMDevEnumerator);
-    MMDevEnumerator = NULL;
 }
 
 static HRESULT WINAPI MMDevEnum_QueryInterface(IMMDeviceEnumerator *iface, REFIID riid, void **ppv)
@@ -910,20 +900,12 @@ static HRESULT WINAPI MMDevEnum_QueryInterface(IMMDeviceEnumerator *iface, REFII
 
 static ULONG WINAPI MMDevEnum_AddRef(IMMDeviceEnumerator *iface)
 {
-    MMDevEnumImpl *This = impl_from_IMMDeviceEnumerator(iface);
-    LONG ref = InterlockedIncrement(&This->ref);
-    TRACE("Refcount now %i\n", ref);
-    return ref;
+    return 2;
 }
 
 static ULONG WINAPI MMDevEnum_Release(IMMDeviceEnumerator *iface)
 {
-    MMDevEnumImpl *This = impl_from_IMMDeviceEnumerator(iface);
-    LONG ref = InterlockedDecrement(&This->ref);
-    if (!ref)
-        MMDevEnum_Free();
-    TRACE("Refcount now %i\n", ref);
-    return ref;
+    return 1;
 }
 
 static HRESULT WINAPI MMDevEnum_EnumAudioEndpoints(IMMDeviceEnumerator *iface, EDataFlow flow, DWORD mask, IMMDeviceCollection **devices)
@@ -1287,6 +1269,12 @@ static const IMMDeviceEnumeratorVtbl MMDevEnumVtbl =
     MMDevEnum_UnregisterEndpointNotificationCallback
 };
 
+static MMDevEnumImpl enumerator =
+{
+    {&MMDevEnumVtbl},
+    FALSE,
+};
+
 static HRESULT MMDevPropStore_Create(MMDevice *parent, DWORD access, IPropertyStore **ppv)
 {
     MMDevPropStore *This;
-- 
2.31.0

[PATCH v2 2/3] mmdevapi: Load devices on driver initialization.

[Rémi Bernon]

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/mmdevapi/devenum.c  | 14 ++------------
 dlls/mmdevapi/main.c     |  6 ++++++
 dlls/mmdevapi/mmdevapi.h |  3 +++
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c
index a5863af2705..d3272a1ff93 100644
--- a/dlls/mmdevapi/devenum.c
+++ b/dlls/mmdevapi/devenum.c
@@ -56,7 +56,6 @@ typedef struct MMDevPropStoreImpl
 typedef struct MMDevEnumImpl
 {
     IMMDeviceEnumerator IMMDeviceEnumerator_iface;
-    BOOL initialized;
 } MMDevEnumImpl;
 
 static MMDevice **MMDevice_head;
@@ -373,7 +372,7 @@ static MMDevice *MMDevice_Create(WCHAR *name, GUID *id, EDataFlow flow, DWORD st
     return cur;
 }
 
-static HRESULT load_devices_from_reg(void)
+HRESULT load_devices_from_reg(void)
 {
     DWORD i = 0;
     HKEY root, cur;
@@ -466,7 +465,7 @@ static HRESULT set_format(MMDevice *dev)
     return S_OK;
 }
 
-static HRESULT load_driver_devices(EDataFlow flow)
+HRESULT load_driver_devices(EDataFlow flow)
 {
     WCHAR **ids;
     GUID *guids;
@@ -860,14 +859,6 @@ static const IMMDeviceCollectionVtbl MMDevColVtbl =
 
 HRESULT MMDevEnum_Create(REFIID riid, void **ppv)
 {
-    if (!enumerator.initialized)
-    {
-        enumerator.initialized = TRUE;
-        load_devices_from_reg();
-        load_driver_devices(eRender);
-        load_driver_devices(eCapture);
-    }
-
     return IMMDeviceEnumerator_QueryInterface(&enumerator.IMMDeviceEnumerator_iface, riid, ppv);
 }
 
@@ -1272,7 +1263,6 @@ static const IMMDeviceEnumeratorVtbl MMDevEnumVtbl =
 static MMDevEnumImpl enumerator =
 {
     {&MMDevEnumVtbl},
-    FALSE,
 };
 
 static HRESULT MMDevPropStore_Create(MMDevice *parent, DWORD access, IPropertyStore **ppv)
diff --git a/dlls/mmdevapi/main.c b/dlls/mmdevapi/main.c
index 9cc0aad6105..358c1697a23 100644
--- a/dlls/mmdevapi/main.c
+++ b/dlls/mmdevapi/main.c
@@ -148,6 +148,12 @@ static BOOL WINAPI init_driver(INIT_ONCE *once, void *param, void **context)
             *next = ',';
     }
 
+    if (drvs.module != 0){
+        load_devices_from_reg();
+        load_driver_devices(eRender);
+        load_driver_devices(eCapture);
+    }
+
     return drvs.module != 0;
 }
 
diff --git a/dlls/mmdevapi/mmdevapi.h b/dlls/mmdevapi/mmdevapi.h
index 3bcf568cddf..73c9b0e9592 100644
--- a/dlls/mmdevapi/mmdevapi.h
+++ b/dlls/mmdevapi/mmdevapi.h
@@ -73,4 +73,7 @@ extern HRESULT AudioClient_Create(MMDevice *parent, IAudioClient **ppv) DECLSPEC
 extern HRESULT AudioEndpointVolume_Create(MMDevice *parent, IAudioEndpointVolumeEx **ppv) DECLSPEC_HIDDEN;
 extern HRESULT SpatialAudioClient_Create(IMMDevice *device, ISpatialAudioClient **out) DECLSPEC_HIDDEN;
 
+extern HRESULT load_devices_from_reg(void) DECLSPEC_HIDDEN;
+extern HRESULT load_driver_devices(EDataFlow flow) DECLSPEC_HIDDEN;
+
 extern const WCHAR drv_keyW[] DECLSPEC_HIDDEN;
-- 
2.31.0

[PATCH v2 2/3] mmdevapi: Load devices on driver initialization.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=91752

Your paranoid android.


=== debiant2 (32 bit report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (32 bit Chinese:China report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (32 bit WoW report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (64 bit WoW report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

[PATCH v2 3/3] mmdevapi: Use a struct list to keep device list.

[Rémi Bernon]

It's not guarded but devices are only created on driver initialization
and destroyed on DLL detach.

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/mmdevapi/devenum.c  | 48 ++++++++++++++--------------------------
 dlls/mmdevapi/mmdevapi.h |  4 ++++
 2 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c
index d3272a1ff93..214ef0eb039 100644
--- a/dlls/mmdevapi/devenum.c
+++ b/dlls/mmdevapi/devenum.c
@@ -58,9 +58,7 @@ typedef struct MMDevEnumImpl
     IMMDeviceEnumerator IMMDeviceEnumerator_iface;
 } MMDevEnumImpl;
 
-static MMDevice **MMDevice_head;
 static MMDevice *MMDevice_def_rec, *MMDevice_def_play;
-static DWORD MMDevice_count;
 static const IMMDeviceEnumeratorVtbl MMDevEnumVtbl;
 static const IMMDeviceCollectionVtbl MMDevColVtbl;
 static const IMMDeviceVtbl MMDeviceVtbl;
@@ -68,6 +66,7 @@ static const IPropertyStoreVtbl MMDevPropVtbl;
 static const IMMEndpointVtbl MMEndpointVtbl;
 
 static MMDevEnumImpl enumerator;
+static struct list device_list = LIST_INIT(device_list);
 static IMMDevice info_device;
 
 typedef struct MMDevColImpl
@@ -262,9 +261,8 @@ static HRESULT set_driver_prop_value(GUID *id, const EDataFlow flow, const PROPE
 static MMDevice *MMDevice_Create(WCHAR *name, GUID *id, EDataFlow flow, DWORD state, BOOL setdefault)
 {
     HKEY key, root;
-    MMDevice *cur = NULL;
+    MMDevice *device, *cur = NULL;
     WCHAR guidstr[39];
-    DWORD i;
 
     static const PROPERTYKEY deviceinterface_key = {
         {0x233164c8, 0x1b2c, 0x4c7d, {0xbc, 0x68, 0xb6, 0x71, 0x68, 0x7a, 0x25, 0x67}}, 1
@@ -274,9 +272,8 @@ static MMDevice *MMDevice_Create(WCHAR *name, GUID *id, EDataFlow flow, DWORD st
         {0xb3f8fa53, 0x0004, 0x438e, {0x90, 0x03, 0x51, 0xa4, 0x6e, 0x13, 0x9b, 0xfc}}, 2
     };
 
-    for (i = 0; i < MMDevice_count; ++i)
+    LIST_FOR_EACH_ENTRY(device, &device_list, MMDevice, entry)
     {
-        MMDevice *device = MMDevice_head[i];
         if (device->flow == flow && IsEqualGUID(&device->devguid, id)){
             cur = device;
             break;
@@ -295,11 +292,7 @@ static MMDevice *MMDevice_Create(WCHAR *name, GUID *id, EDataFlow flow, DWORD st
         InitializeCriticalSection(&cur->crst);
         cur->crst.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": MMDevice.crst");
 
-        if (!MMDevice_head)
-            MMDevice_head = HeapAlloc(GetProcessHeap(), 0, sizeof(*MMDevice_head));
-        else
-            MMDevice_head = HeapReAlloc(GetProcessHeap(), 0, MMDevice_head, sizeof(*MMDevice_head)*(1+MMDevice_count));
-        MMDevice_head[MMDevice_count++] = cur;
+        list_add_tail(&device_list, &cur->entry);
     }else if(cur->ref > 0)
         WARN("Modifying an MMDevice with postitive reference count!\n");
 
@@ -494,17 +487,8 @@ HRESULT load_driver_devices(EDataFlow flow)
 
 static void MMDevice_Destroy(MMDevice *This)
 {
-    DWORD i;
     TRACE("Freeing %s\n", debugstr_w(This->drv_id));
-    /* Since this function is called at destruction time, reordering of the list is unimportant */
-    for (i = 0; i < MMDevice_count; ++i)
-    {
-        if (MMDevice_head[i] == This)
-        {
-            MMDevice_head[i] = MMDevice_head[--MMDevice_count];
-            break;
-        }
-    }
+    list_remove(&This->entry);
     This->crst.DebugInfo->Spare[0] = 0;
     DeleteCriticalSection(&This->crst);
     HeapFree(GetProcessHeap(), 0, This->drv_id);
@@ -805,16 +789,15 @@ static ULONG WINAPI MMDevCol_Release(IMMDeviceCollection *iface)
 static HRESULT WINAPI MMDevCol_GetCount(IMMDeviceCollection *iface, UINT *numdevs)
 {
     MMDevColImpl *This = impl_from_IMMDeviceCollection(iface);
-    DWORD i;
+    MMDevice *cur;
 
     TRACE("(%p)->(%p)\n", This, numdevs);
     if (!numdevs)
         return E_POINTER;
 
     *numdevs = 0;
-    for (i = 0; i < MMDevice_count; ++i)
+    LIST_FOR_EACH_ENTRY(cur, &device_list, MMDevice, entry)
     {
-        MMDevice *cur = MMDevice_head[i];
         if ((cur->flow == This->flow || This->flow == eAll)
             && (cur->state & This->state))
             ++(*numdevs);
@@ -825,15 +808,15 @@ static HRESULT WINAPI MMDevCol_GetCount(IMMDeviceCollection *iface, UINT *numdev
 static HRESULT WINAPI MMDevCol_Item(IMMDeviceCollection *iface, UINT n, IMMDevice **dev)
 {
     MMDevColImpl *This = impl_from_IMMDeviceCollection(iface);
-    DWORD i = 0, j = 0;
+    MMDevice *cur;
+    DWORD i = 0;
 
     TRACE("(%p)->(%u, %p)\n", This, n, dev);
     if (!dev)
         return E_POINTER;
 
-    for (j = 0; j < MMDevice_count; ++j)
+    LIST_FOR_EACH_ENTRY(cur, &device_list, MMDevice, entry)
     {
-        MMDevice *cur = MMDevice_head[j];
         if ((cur->flow == This->flow || This->flow == eAll)
             && (cur->state & This->state)
             && i++ == n)
@@ -864,8 +847,9 @@ HRESULT MMDevEnum_Create(REFIID riid, void **ppv)
 
 void MMDevEnum_Free(void)
 {
-    while (MMDevice_count)
-        MMDevice_Destroy(MMDevice_head[0]);
+    MMDevice *device, *next;
+    LIST_FOR_EACH_ENTRY_SAFE(device, next, &device_list, MMDevice, entry)
+        MMDevice_Destroy(device);
     RegCloseKey(key_render);
     RegCloseKey(key_capture);
     key_render = key_capture = NULL;
@@ -999,7 +983,7 @@ static HRESULT WINAPI MMDevEnum_GetDefaultAudioEndpoint(IMMDeviceEnumerator *ifa
 static HRESULT WINAPI MMDevEnum_GetDevice(IMMDeviceEnumerator *iface, const WCHAR *name, IMMDevice **device)
 {
     MMDevEnumImpl *This = impl_from_IMMDeviceEnumerator(iface);
-    DWORD i=0;
+    MMDevice *impl;
     IMMDevice *dev = NULL;
 
     TRACE("(%p)->(%s,%p)\n", This, debugstr_w(name), device);
@@ -1012,11 +996,11 @@ static HRESULT WINAPI MMDevEnum_GetDevice(IMMDeviceEnumerator *iface, const WCHA
         return S_OK;
     }
 
-    for (i = 0; i < MMDevice_count; ++i)
+    LIST_FOR_EACH_ENTRY(impl, &device_list, MMDevice, entry)
     {
         HRESULT hr;
         WCHAR *str;
-        dev = &MMDevice_head[i]->IMMDevice_iface;
+        dev = &impl->IMMDevice_iface;
         hr = IMMDevice_GetId(dev, &str);
         if (FAILED(hr))
         {
diff --git a/dlls/mmdevapi/mmdevapi.h b/dlls/mmdevapi/mmdevapi.h
index 73c9b0e9592..db91e333cc8 100644
--- a/dlls/mmdevapi/mmdevapi.h
+++ b/dlls/mmdevapi/mmdevapi.h
@@ -16,6 +16,8 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 
+#include <wine/list.h>
+
 extern HRESULT MMDevEnum_Create(REFIID riid, void **ppv) DECLSPEC_HIDDEN;
 extern void MMDevEnum_Free(void) DECLSPEC_HIDDEN;
 
@@ -67,6 +69,8 @@ typedef struct MMDevice {
     DWORD state;
     GUID devguid;
     WCHAR *drv_id;
+
+    struct list entry;
 } MMDevice;
 
 extern HRESULT AudioClient_Create(MMDevice *parent, IAudioClient **ppv) DECLSPEC_HIDDEN;
-- 
2.31.0

[PATCH v2 3/3] mmdevapi: Use a struct list to keep device list.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=91753

Your paranoid android.


=== debiant2 (32 bit report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (32 bit Chinese:China report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (32 bit WoW report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (64 bit WoW report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

[PATCH v2 3/3] mmdevapi: Use a struct list to keep device list.

[Rémi Bernon]

On 6/3/21 3:54 PM, Marvin wrote:
> Hi,
>
> While running your changed tests, I think I found new failures.
> Being a bot and all I'm not very good at pattern recognition, so I might be
> wrong, but could you please double-check?
>
> Full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=91753
>
> Your paranoid android.
>
> === debiant2 (32 bit report) ===
>
> mmdevapi:
> mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
> mmdevenum.c:54: Test failed: Reference count on parent is 1
>
> === debiant2 (32 bit Chinese:China report) ===
>
> mmdevapi:
> mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
> mmdevenum.c:54: Test failed: Reference count on parent is 1
>
> === debiant2 (32 bit WoW report) ===
>
> mmdevapi:
> mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
> mmdevenum.c:54: Test failed: Reference count on parent is 1
>
> === debiant2 (64 bit WoW report) ===
>
> mmdevapi:
> mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
> mmdevenum.c:54: Test failed: Reference count on parent is 1

Would it be alright to remove these tests? Is it important to keep the 
refcounting for a static COM object?
-- 
Rémi Bernon <rbernon(a)codeweavers.com>

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=91751

Your paranoid android.


=== debiant2 (32 bit report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (32 bit Chinese:China report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (32 bit WoW report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1

=== debiant2 (64 bit WoW report) ===

mmdevapi:
mmdevenum.c:405: Test failed: Invalid reference count after incrementing: 2
mmdevenum.c:54: Test failed: Reference count on parent is 1