[PATCH v2 0/1] MR8949: dxcore: Avoid races with the global adapter factory

[Tim Clem (＠tclem)]

If multiple threads are creating and releasing the factory, these
accesses are racy and can result in callers of
DXCoreCreateAdapterFactory getting a null pointer or a pointer to
freed memory.

--
  v2: dxcore: Use a static structure for the adapter factory.

https://gitlab.winehq.org/wine/wine/-/merge_requests/8949

[PATCH v2 1/1] dxcore: Use a static structure for the adapter factory.

[Tim Clem]

From: Tim Clem

To avoid races with the dynamically allocated approach.
---
 dlls/dxcore/dxcore.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/dlls/dxcore/dxcore.c b/dlls/dxcore/dxcore.c
index 71cdca9412a..a63a8f33d72 100644
--- a/dlls/dxcore/dxcore.c
+++ b/dlls/dxcore/dxcore.c
@@ -27,8 +27,6 @@

WINE_DEFAULT_DEBUG_CHANNEL(dxcore);

-static struct dxcore_adapter_factory *dxcore_adapter_factory;
-
 struct dxcore_adapter
 {
     IDXCoreAdapter IDXCoreAdapter_iface;
@@ -442,12 +440,6 @@ static ULONG STDMETHODCALLTYPE dxcore_adapter_factory_Release(IDXCoreAdapterFact

TRACE("%p decreasing refcount to %lu.\n", iface, refcount);

-    if (!refcount)
-    {
-        free(factory);
-        dxcore_adapter_factory = NULL;
-    }
-
     return refcount;
 }

@@ -612,25 +604,23 @@ static const struct IDXCoreAdapterFactoryVtbl dxcore_adapter_factory_vtbl =
     dxcore_adapter_factory_UnregisterEventNotification,
 };

+static struct dxcore_adapter_factory dxcore_adapter_factory = {
+    .IDXCoreAdapterFactory_iface.lpVtbl = &dxcore_adapter_factory_vtbl,
+    .refcount = 0
+};
+
 HRESULT STDMETHODCALLTYPE DXCoreCreateAdapterFactory(REFIID riid, void **out)
 {
+    HRESULT hr;
+
     TRACE("riid %s, out %p\n", debugstr_guid(riid), out);

if (!out)
         return E_POINTER;

-    if (!dxcore_adapter_factory)
-    {
-        if (!(dxcore_adapter_factory = calloc(1, sizeof(*dxcore_adapter_factory))))
-        {
-            *out = NULL;
-            return E_OUTOFMEMORY;
-        }
+    hr = IDXCoreAdapterFactory_QueryInterface(&dxcore_adapter_factory.IDXCoreAdapterFactory_iface, riid, out);
+    if (SUCCEEDED(hr))
+        TRACE("returning factory %p\n", *out);

-        dxcore_adapter_factory->IDXCoreAdapterFactory_iface.lpVtbl = &dxcore_adapter_factory_vtbl;
-        dxcore_adapter_factory->refcount = 0;
-    }
-
-    TRACE("created IDXCoreAdapterFactory %p.\n", *out);
-    return IDXCoreAdapterFactory_QueryInterface(&dxcore_adapter_factory->IDXCoreAdapterFactory_iface, riid, out);
+    return hr;
 }

[Tim Clem (＠tclem)]

On Wed Sep 10 18:40:24 2025 +0000, Elizabeth Figura wrote:
> The refcount should stay I think; it just won't free anything on release.

How's v2 look? Also fixes the issue Nikolay mentioned with invalid IIDs.

[PATCH v2 0/1] MR8949: dxcore: Avoid races with the global adapter factory - approved

[Elizabeth Figura (＠zfigura)]

This merge request was approved by Elizabeth Figura.