[PATCH vkd3d 2/3] vkd3d: Protect private data with mutex.
From: Józef Kucia <jkucia(a)codeweavers.com> Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- libs/vkd3d/command.c | 37 +++++++++++++++++++++------ 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, 155 insertions(+), 33 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 4bf11d4b9997..986efb2d3b63 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,11 @@ 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(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
On Wed, 9 Jan 2019 at 16:53, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
@@ -4934,7 +4953,11 @@ 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(object); + return hr; + } Does this leak "object->desc.pArgumentDescs" on failure?
On Wed, Jan 9, 2019 at 9:02 PM Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Wed, 9 Jan 2019 at 16:53, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
@@ -4934,7 +4953,11 @@ 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(object); + return hr; + } Does this leak "object->desc.pArgumentDescs" on failure?
Yes, Thanks for catching this.
participants (2)
-
Henri Verbeet -
Józef Kucia