[PATCH vkd3d v2 2/3] vkd3d: Protect private data with mutex.

[Józef Kucia]

From: Józef Kucia <jkucia(a)codeweavers.com>

Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com>
---

Version 2: Fix memory leak in d3d12_command_signature_create().

---
 libs/vkd3d/command.c       | 38 ++++++++++++++++++++++------
 libs/vkd3d/device.c        |  9 ++++---
 libs/vkd3d/resource.c      | 51 +++++++++++++++++++++++++++++++-------
 libs/vkd3d/state.c         | 20 ++++++++++++---
 libs/vkd3d/utils.c         | 50 ++++++++++++++++++++++++++++++++-----
 libs/vkd3d/vkd3d_private.h | 21 ++++++++++++----
 6 files changed, 156 insertions(+), 33 deletions(-)

diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c
index 4bf11d4b9997..ce4af4e6a6fd 100644
--- a/libs/vkd3d/command.c
+++ b/libs/vkd3d/command.c
@@ -578,6 +578,7 @@ static const struct ID3D12FenceVtbl d3d12_fence_vtbl =
 static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device *device,
         UINT64 initial_value, D3D12_FENCE_FLAGS flags)
 {
+    HRESULT hr;
     int rc;
 
     fence->ID3D12Fence_iface.lpVtbl = &d3d12_fence_vtbl;
@@ -598,7 +599,11 @@ static HRESULT d3d12_fence_init(struct d3d12_fence *fence, struct d3d12_device *
     fence->events_size = 0;
     fence->event_count = 0;
 
-    vkd3d_private_store_init(&fence->private_store);
+    if (FAILED(hr = vkd3d_private_store_init(&fence->private_store)))
+    {
+        pthread_mutex_destroy(&fence->mutex);
+        return hr;
+    }
 
     fence->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
@@ -1182,6 +1187,10 @@ static HRESULT d3d12_command_allocator_init(struct d3d12_command_allocator *allo
     VkCommandPoolCreateInfo command_pool_info;
     struct vkd3d_queue *queue;
     VkResult vr;
+    HRESULT hr;
+
+    if (FAILED(hr = vkd3d_private_store_init(&allocator->private_store)))
+        return hr;
 
     if (!(queue = d3d12_device_get_vkd3d_queue(device, type)))
         queue = device->direct_queue;
@@ -1201,6 +1210,7 @@ static HRESULT d3d12_command_allocator_init(struct d3d12_command_allocator *allo
             &allocator->vk_command_pool))) < 0)
     {
         WARN("Failed to create Vulkan command pool, vr %d.\n", vr);
+        vkd3d_private_store_destroy(&allocator->private_store);
         return hresult_from_vk_result(vr);
     }
 
@@ -1240,8 +1250,6 @@ static HRESULT d3d12_command_allocator_init(struct d3d12_command_allocator *allo
 
     allocator->current_command_list = NULL;
 
-    vkd3d_private_store_init(&allocator->private_store);
-
     allocator->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
 
@@ -4287,17 +4295,24 @@ static HRESULT d3d12_command_list_init(struct d3d12_command_list *list, struct d
     list->refcount = 1;
 
     list->type = type;
+
+    if (FAILED(hr = vkd3d_private_store_init(&list->private_store)))
+        return hr;
+
     list->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
 
-    vkd3d_private_store_init(&list->private_store);
-
     list->allocator = allocator;
 
     if (SUCCEEDED(hr = d3d12_command_allocator_allocate_command_buffer(allocator, list)))
+    {
         d3d12_command_list_reset_state(list, initial_pipeline_state);
+    }
     else
+    {
+        vkd3d_private_store_destroy(&list->private_store);
         ID3D12Device_Release(&device->ID3D12Device_iface);
+    }
 
     return hr;
 }
@@ -4696,6 +4711,8 @@ static const struct ID3D12CommandQueueVtbl d3d12_command_queue_vtbl =
 static HRESULT d3d12_command_queue_init(struct d3d12_command_queue *queue,
         struct d3d12_device *device, const D3D12_COMMAND_QUEUE_DESC *desc)
 {
+    HRESULT hr;
+
     queue->ID3D12CommandQueue_iface.lpVtbl = &d3d12_command_queue_vtbl;
     queue->refcount = 1;
 
@@ -4711,7 +4728,8 @@ static HRESULT d3d12_command_queue_init(struct d3d12_command_queue *queue,
     if (desc->Flags)
         FIXME("Ignoring flags %#x.\n", desc->Flags);
 
-    vkd3d_private_store_init(&queue->private_store);
+    if (FAILED(hr = vkd3d_private_store_init(&queue->private_store)))
+        return hr;
 
     queue->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
@@ -4899,6 +4917,7 @@ HRESULT d3d12_command_signature_create(struct d3d12_device *device, const D3D12_
 {
     struct d3d12_command_signature *object;
     unsigned int i;
+    HRESULT hr;
 
     for (i = 0; i < desc->NumArgumentDescs; ++i)
     {
@@ -4934,7 +4953,12 @@ HRESULT d3d12_command_signature_create(struct d3d12_device *device, const D3D12_
     memcpy((void *)object->desc.pArgumentDescs, desc->pArgumentDescs,
             desc->NumArgumentDescs * sizeof(*desc->pArgumentDescs));
 
-    vkd3d_private_store_init(&object->private_store);
+    if (FAILED(hr = vkd3d_private_store_init(&object->private_store)))
+    {
+        vkd3d_free((void *)object->desc.pArgumentDescs);
+        vkd3d_free(object);
+        return hr;
+    }
 
     object->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c
index 7a7f4aaa143c..9b09f66b7251 100644
--- a/libs/vkd3d/device.c
+++ b/libs/vkd3d/device.c
@@ -2526,18 +2526,21 @@ static HRESULT d3d12_device_init(struct d3d12_device *device,
     if (FAILED(hr = d3d12_device_init_pipeline_cache(device)))
         goto out_free_vk_resources;
 
-    if (FAILED(hr = vkd3d_fence_worker_start(&device->fence_worker, device)))
+    if (FAILED(hr = vkd3d_private_store_init(&device->private_store)))
         goto out_free_pipeline_cache;
 
+    if (FAILED(hr = vkd3d_fence_worker_start(&device->fence_worker, device)))
+        goto out_free_private_store;
+
     vkd3d_gpu_va_allocator_init(&device->gpu_va_allocator);
 
     if ((device->parent = create_info->parent))
         IUnknown_AddRef(device->parent);
 
-    vkd3d_private_store_init(&device->private_store);
-
     return S_OK;
 
+out_free_private_store:
+    vkd3d_private_store_destroy(&device->private_store);
 out_free_pipeline_cache:
     d3d12_device_destroy_pipeline_cache(device);
 out_free_vk_resources:
diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c
index ffce926e476a..5b4110886299 100644
--- a/libs/vkd3d/resource.c
+++ b/libs/vkd3d/resource.c
@@ -379,15 +379,20 @@ static HRESULT d3d12_heap_init(struct d3d12_heap *heap,
         return hresult_from_errno(rc);
     }
 
+    if (FAILED(hr = vkd3d_private_store_init(&heap->private_store)))
+    {
+        pthread_mutex_destroy(&heap->mutex);
+        return hr;
+    }
+
     if (FAILED(hr = vkd3d_allocate_device_memory(device, &heap->desc.Properties,
             heap->desc.Flags, &memory_requirements, &heap->vk_memory, &heap->vk_memory_type)))
     {
+        vkd3d_private_store_destroy(&heap->private_store);
         pthread_mutex_destroy(&heap->mutex);
         return hr;
     }
 
-    vkd3d_private_store_init(&heap->private_store);
-
     heap->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
 
@@ -1179,7 +1184,11 @@ static HRESULT d3d12_resource_init(struct d3d12_resource *resource, struct d3d12
     resource->heap = NULL;
     resource->heap_offset = 0;
 
-    vkd3d_private_store_init(&resource->private_store);
+    if (FAILED(hr = vkd3d_private_store_init(&resource->private_store)))
+    {
+        d3d12_resource_destroy(resource, device);
+        return hr;
+    }
 
     resource->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
@@ -1325,6 +1334,7 @@ HRESULT vkd3d_create_image_resource(ID3D12Device *device,
 {
     struct d3d12_device *d3d12_device = unsafe_impl_from_ID3D12Device(device);
     struct d3d12_resource *object;
+    HRESULT hr;
 
     TRACE("device %p, create_info %p, resource %p.\n", device, create_info, resource);
 
@@ -1357,9 +1367,15 @@ HRESULT vkd3d_create_image_resource(ID3D12Device *device,
         object->present_state = create_info->present_state;
     else
         object->present_state = D3D12_RESOURCE_STATE_COMMON;
+
+    if (FAILED(hr = vkd3d_private_store_init(&object->private_store)))
+    {
+        vkd3d_free(object);
+        return hr;
+    }
+
     object->device = d3d12_device;
     ID3D12Device_AddRef(&d3d12_device->ID3D12Device_iface);
-    vkd3d_private_store_init(&object->private_store);
 
     TRACE("Created resource %p.\n", object);
 
@@ -2526,18 +2542,23 @@ static const struct ID3D12DescriptorHeapVtbl d3d12_descriptor_heap_vtbl =
     d3d12_descriptor_heap_GetGPUDescriptorHandleForHeapStart,
 };
 
-static void d3d12_descriptor_heap_init(struct d3d12_descriptor_heap *descriptor_heap,
+static HRESULT d3d12_descriptor_heap_init(struct d3d12_descriptor_heap *descriptor_heap,
         struct d3d12_device *device, const D3D12_DESCRIPTOR_HEAP_DESC *desc)
 {
+    HRESULT hr;
+
     descriptor_heap->ID3D12DescriptorHeap_iface.lpVtbl = &d3d12_descriptor_heap_vtbl;
     descriptor_heap->refcount = 1;
 
     descriptor_heap->desc = *desc;
 
-    vkd3d_private_store_init(&descriptor_heap->private_store);
+    if (FAILED(hr = vkd3d_private_store_init(&descriptor_heap->private_store)))
+        return hr;
 
     descriptor_heap->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
+
+    return S_OK;
 }
 
 HRESULT d3d12_descriptor_heap_create(struct d3d12_device *device,
@@ -2545,6 +2566,7 @@ HRESULT d3d12_descriptor_heap_create(struct d3d12_device *device,
 {
     size_t max_descriptor_count, descriptor_size;
     struct d3d12_descriptor_heap *object;
+    HRESULT hr;
 
     if (!(descriptor_size = ID3D12Device_GetDescriptorHandleIncrementSize(&device->ID3D12Device_iface, desc->Type)))
     {
@@ -2570,7 +2592,12 @@ HRESULT d3d12_descriptor_heap_create(struct d3d12_device *device,
             descriptors[descriptor_size * desc->NumDescriptors]))))
         return E_OUTOFMEMORY;
 
-    d3d12_descriptor_heap_init(object, device, desc);
+    if (FAILED(hr = d3d12_descriptor_heap_init(object, device, desc)))
+    {
+        vkd3d_free(object);
+        return hr;
+    }
+
     memset(object->descriptors, 0, descriptor_size * desc->NumDescriptors);
 
     TRACE("Created descriptor heap %p.\n", object);
@@ -2722,6 +2749,7 @@ HRESULT d3d12_query_heap_create(struct d3d12_device *device, const D3D12_QUERY_H
     VkQueryPoolCreateInfo pool_info;
     unsigned int element_count;
     VkResult vr;
+    HRESULT hr;
 
     element_count = DIV_ROUND_UP(desc->Count, sizeof(*object->availability_mask) * CHAR_BIT);
     if (!(object = vkd3d_malloc(offsetof(struct d3d12_query_heap, availability_mask[element_count]))))
@@ -2775,15 +2803,20 @@ HRESULT d3d12_query_heap_create(struct d3d12_device *device, const D3D12_QUERY_H
             return E_INVALIDARG;
     }
 
+    if (FAILED(hr = vkd3d_private_store_init(&object->private_store)))
+    {
+        vkd3d_free(object);
+        return hr;
+    }
+
     if ((vr = VK_CALL(vkCreateQueryPool(device->vk_device, &pool_info, NULL, &object->vk_query_pool))) < 0)
     {
         WARN("Failed to create Vulkan query pool, vr %d.\n", vr);
+        vkd3d_private_store_destroy(&object->private_store);
         vkd3d_free(object);
         return hresult_from_vk_result(vr);
     }
 
-    vkd3d_private_store_init(&object->private_store);
-
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
 
     TRACE("Created query heap %p.\n", object);
diff --git a/libs/vkd3d/state.c b/libs/vkd3d/state.c
index ee91c725f75d..771ef241738a 100644
--- a/libs/vkd3d/state.c
+++ b/libs/vkd3d/state.c
@@ -999,7 +999,8 @@ static HRESULT d3d12_root_signature_init(struct d3d12_root_signature *root_signa
             &root_signature->vk_pipeline_layout)))
         goto fail;
 
-    vkd3d_private_store_init(&root_signature->private_store);
+    if (FAILED(hr = vkd3d_private_store_init(&root_signature->private_store)))
+        goto fail;
 
     root_signature->device = device;
     ID3D12Device_AddRef(&device->ID3D12Device_iface);
@@ -1516,7 +1517,16 @@ static HRESULT d3d12_pipeline_state_init_compute(struct d3d12_pipeline_state *st
         return hresult_from_vk_result(vr);
     }
 
-    vkd3d_private_store_init(&state->private_store);
+    if (FAILED(hr = vkd3d_private_store_init(&state->private_store)))
+    {
+        VK_CALL(vkDestroyPipeline(device->vk_device, state->u.compute.vk_pipeline, NULL));
+        if (state->vk_set_layout)
+            VK_CALL(vkDestroyDescriptorSetLayout(device->vk_device, state->vk_set_layout, NULL));
+        if (state->vk_pipeline_layout)
+            VK_CALL(vkDestroyPipelineLayout(device->vk_device, state->vk_pipeline_layout, NULL));
+        vkd3d_free(state->uav_counters);
+        return hr;
+    }
 
     state->vk_bind_point = VK_PIPELINE_BIND_POINT_COMPUTE;
     state->device = device;
@@ -2359,7 +2369,11 @@ static HRESULT d3d12_pipeline_state_init_graphics(struct d3d12_pipeline_state *s
 
     list_init(&graphics->compiled_pipelines);
 
-    vkd3d_private_store_init(&state->private_store);
+    if (FAILED(hr = vkd3d_private_store_init(&state->private_store)))
+    {
+        VK_CALL(vkDestroyRenderPass(device->vk_device, graphics->render_pass, NULL));
+        goto fail;
+    }
 
     state->vk_bind_point = VK_PIPELINE_BIND_POINT_GRAPHICS;
     state->device = device;
diff --git a/libs/vkd3d/utils.c b/libs/vkd3d/utils.c
index ed4d77fea8cd..0144fdc1bd40 100644
--- a/libs/vkd3d/utils.c
+++ b/libs/vkd3d/utils.c
@@ -566,41 +566,79 @@ HRESULT vkd3d_get_private_data(struct vkd3d_private_store *store,
         const GUID *tag, unsigned int *out_size, void *out)
 {
     const struct vkd3d_private_data *data;
+    HRESULT hr = S_OK;
     unsigned int size;
+    int rc;
 
     if (!out_size)
         return E_INVALIDARG;
 
+    if ((rc = pthread_mutex_lock(&store->mutex)))
+    {
+        ERR("Failed to lock mutex, error %d.\n", rc);
+        return hresult_from_errno(rc);
+    }
+
     if (!(data = vkd3d_private_store_get_private_data(store, tag)))
     {
         *out_size = 0;
-        return DXGI_ERROR_NOT_FOUND;
+        hr = DXGI_ERROR_NOT_FOUND;
+        goto done;
     }
 
     size = *out_size;
     *out_size = data->size;
     if (!out)
-        return S_OK;
+        goto done;
 
     if (size < data->size)
-        return DXGI_ERROR_MORE_DATA;
+    {
+        hr = DXGI_ERROR_MORE_DATA;
+        goto done;
+    }
 
     if (data->is_object)
         IUnknown_AddRef(data->u.object);
     memcpy(out, data->u.data, data->size);
 
-    return S_OK;
+done:
+    pthread_mutex_unlock(&store->mutex);
+    return hr;
 }
 
 HRESULT vkd3d_set_private_data(struct vkd3d_private_store *store,
         const GUID *tag, unsigned int data_size, const void *data)
 {
-    return vkd3d_private_store_set_private_data(store, tag, data, data_size, false);
+    HRESULT hr;
+    int rc;
+
+    if ((rc = pthread_mutex_lock(&store->mutex)))
+    {
+        ERR("Failed to lock mutex, error %d.\n", rc);
+        return hresult_from_errno(rc);
+    }
+
+    hr = vkd3d_private_store_set_private_data(store, tag, data, data_size, false);
+
+    pthread_mutex_unlock(&store->mutex);
+    return hr;
 }
 
 HRESULT vkd3d_set_private_data_interface(struct vkd3d_private_store *store,
         const GUID *tag, const IUnknown *object)
 {
     const void *data = object ? object : (void *)&object;
-    return vkd3d_private_store_set_private_data(store, tag, data, sizeof(object), !!object);
+    HRESULT hr;
+    int rc;
+
+    if ((rc = pthread_mutex_lock(&store->mutex)))
+    {
+        ERR("Failed to lock mutex, error %d.\n", rc);
+        return hresult_from_errno(rc);
+    }
+
+    hr = vkd3d_private_store_set_private_data(store, tag, data, sizeof(object), !!object);
+
+    pthread_mutex_unlock(&store->mutex);
+    return hr;
 }
diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h
index c0c15b245902..9ffaa7920eb1 100644
--- a/libs/vkd3d/vkd3d_private.h
+++ b/libs/vkd3d/vkd3d_private.h
@@ -76,6 +76,10 @@ struct vkd3d_vk_device_procs
 };
 #undef DECLARE_VK_PFN
 
+HRESULT hresult_from_errno(int rc) DECLSPEC_HIDDEN;
+HRESULT hresult_from_vk_result(VkResult vr) DECLSPEC_HIDDEN;
+HRESULT hresult_from_vkd3d_result(int vkd3d_result) DECLSPEC_HIDDEN;
+
 struct vkd3d_vulkan_info
 {
     /* instance extensions */
@@ -165,6 +169,8 @@ void vkd3d_gpu_va_allocator_free(struct vkd3d_gpu_va_allocator *allocator,
 
 struct vkd3d_private_store
 {
+    pthread_mutex_t mutex;
+
     struct list content;
 };
 
@@ -190,9 +196,16 @@ static inline void vkd3d_private_data_destroy(struct vkd3d_private_data *data)
     vkd3d_free(data);
 }
 
-static inline void vkd3d_private_store_init(struct vkd3d_private_store *store)
+static inline HRESULT vkd3d_private_store_init(struct vkd3d_private_store *store)
 {
+    int rc;
+
     list_init(&store->content);
+
+    if ((rc = pthread_mutex_init(&store->mutex, NULL)))
+        ERR("Failed to initialize mutex, error %d.\n", rc);
+
+    return hresult_from_errno(rc);
 }
 
 static inline void vkd3d_private_store_destroy(struct vkd3d_private_store *store)
@@ -203,6 +216,8 @@ static inline void vkd3d_private_store_destroy(struct vkd3d_private_store *store
     {
         vkd3d_private_data_destroy(data);
     }
+
+    pthread_mutex_destroy(&store->mutex);
 }
 
 HRESULT vkd3d_get_private_data(struct vkd3d_private_store *store,
@@ -977,10 +992,6 @@ static inline void debug_ignored_node_mask(unsigned int mask)
         FIXME("Ignoring node mask 0x%08x.\n", mask);
 }
 
-HRESULT hresult_from_errno(int rc) DECLSPEC_HIDDEN;
-HRESULT hresult_from_vk_result(VkResult vr) DECLSPEC_HIDDEN;
-HRESULT hresult_from_vkd3d_result(int vkd3d_result) DECLSPEC_HIDDEN;
-
 HRESULT vkd3d_load_vk_global_procs(struct vkd3d_vk_global_procs *procs,
         PFN_vkGetInstanceProcAddr vkGetInstanceProcAddr) DECLSPEC_HIDDEN;
 HRESULT vkd3d_load_vk_instance_procs(struct vkd3d_vk_instance_procs *procs,
-- 
2.19.2

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>