[PATCH] wined3d: Decrement reference count and take the lock atomically for cached objects.
Samplers, blend states, rasterizer states and depth stencil states can be retrieved from the cache in struct d3d_device even after the reference count reaches zero, causing memory corruption.
Signed-off-by: Jan Sikorski jsikorski@codeweavers.com --- dlls/wined3d/sampler.c | 3 +-- dlls/wined3d/state.c | 9 +++------ dlls/wined3d/wined3d_private.h | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/dlls/wined3d/sampler.c b/dlls/wined3d/sampler.c index bba3165316e..fa648304d07 100644 --- a/dlls/wined3d/sampler.c +++ b/dlls/wined3d/sampler.c @@ -35,13 +35,12 @@ ULONG CDECL wined3d_sampler_incref(struct wined3d_sampler *sampler)
ULONG CDECL wined3d_sampler_decref(struct wined3d_sampler *sampler) { - ULONG refcount = InterlockedDecrement(&sampler->refcount); + ULONG refcount = wined3d_atomic_decrement_mutex_lock(&sampler->refcount);
TRACE("%p decreasing refcount to %u.\n", sampler, refcount);
if (!refcount) { - wined3d_mutex_lock(); sampler->parent_ops->wined3d_object_destroyed(sampler->parent); sampler->device->adapter->adapter_ops->adapter_destroy_sampler(sampler); wined3d_mutex_unlock(); diff --git a/dlls/wined3d/state.c b/dlls/wined3d/state.c index c76e0c2b604..87ebefea9d0 100644 --- a/dlls/wined3d/state.c +++ b/dlls/wined3d/state.c @@ -55,14 +55,13 @@ static void wined3d_blend_state_destroy_object(void *object)
ULONG CDECL wined3d_blend_state_decref(struct wined3d_blend_state *state) { - ULONG refcount = InterlockedDecrement(&state->refcount); + ULONG refcount = wined3d_atomic_decrement_mutex_lock(&state->refcount); struct wined3d_device *device = state->device;
TRACE("%p decreasing refcount to %u.\n", state, refcount);
if (!refcount) { - wined3d_mutex_lock(); state->parent_ops->wined3d_object_destroyed(state->parent); wined3d_cs_destroy_object(device->cs, wined3d_blend_state_destroy_object, state); wined3d_mutex_unlock(); @@ -131,14 +130,13 @@ static void wined3d_depth_stencil_state_destroy_object(void *object)
ULONG CDECL wined3d_depth_stencil_state_decref(struct wined3d_depth_stencil_state *state) { - ULONG refcount = InterlockedDecrement(&state->refcount); + ULONG refcount = wined3d_atomic_decrement_mutex_lock(&state->refcount); struct wined3d_device *device = state->device;
TRACE("%p decreasing refcount to %u.\n", state, refcount);
if (!refcount) { - wined3d_mutex_lock(); state->parent_ops->wined3d_object_destroyed(state->parent); wined3d_cs_destroy_object(device->cs, wined3d_depth_stencil_state_destroy_object, state); wined3d_mutex_unlock(); @@ -196,14 +194,13 @@ static void wined3d_rasterizer_state_destroy_object(void *object)
ULONG CDECL wined3d_rasterizer_state_decref(struct wined3d_rasterizer_state *state) { - ULONG refcount = InterlockedDecrement(&state->refcount); + ULONG refcount = wined3d_atomic_decrement_mutex_lock(&state->refcount); struct wined3d_device *device = state->device;
TRACE("%p decreasing refcount to %u.\n", state, refcount);
if (!refcount) { - wined3d_mutex_lock(); state->parent_ops->wined3d_object_destroyed(state->parent); wined3d_cs_destroy_object(device->cs, wined3d_rasterizer_state_destroy_object, state); wined3d_mutex_unlock(); diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 2fce585c6b1..6047defcc2e 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4155,6 +4155,26 @@ const char *wined3d_debug_view_desc(const struct wined3d_view_desc *d, const struct wined3d_resource *resource) DECLSPEC_HIDDEN; const char *wined3d_debug_vkresult(VkResult vr) DECLSPEC_HIDDEN;
+static inline ULONG wined3d_atomic_decrement_mutex_lock(volatile LONG *refcount) +{ + ULONG count, old_count = *refcount; + do + { + if ((count = old_count) == 1) + { + wined3d_mutex_lock(); + count = InterlockedDecrement(refcount); + if (count) wined3d_mutex_unlock(); + return count; + } + + old_count = InterlockedCompareExchange(refcount, count - 1, count); + } + while (old_count != count); + + return count - 1; +} + struct wined3d_client_resource { /* The resource's persistently mapped address, which we may use to perform
On Wed, 1 Dec 2021 at 13:31, Jan Sikorski jsikorski@codeweavers.com wrote:
Samplers, blend states, rasterizer states and depth stencil states can be retrieved from the cache in struct d3d_device even after the reference count reaches zero, causing memory corruption.
I'm signing off on this because it solves a real problem and we're close to code-freeze, but I don't think this is great. Ultimately, d3d11 can't rely on wined3d_mutex_lock()/wined3d_mutex_unlock() to protect its own internal data structures like the state object caches, simply because they're going to go away. That means the interaction between the cache, parent_ops, and locking is going to need some more thought. It may end up being the case that the caches should be handled be wined3d, for example.
-
Henri Verbeet -
Henri Verbeet -
Jan Sikorski