griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix: Vulkan: Unguarded Access Device Queues

Browse Source 
Multiple threads can access the same device queue from different
threads. This could happen when doing a cycles preview render, baking
eevee volume probes or generating material previews.

This PR adds a mutex around access to the device queues.

Detected when researching #128608

Pull Request: https://projects.blender.org/blender/blender/pulls/128974
This commit is contained in:
Jeroen Bakker
2024-10-14 15:30:11 +02:00
parent 2806d54320
commit af151e89a7
9 changed files with 47 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
intern/ghost/GHOST_C-api.h
View File

@@ -1277,13 +1277,17 @@ int GHOST_XrGetControllerModelData(GHOST_XrContextHandle xr_context,
* \param r_queue: After calling this function the VkQueue
* referenced by this parameter will contain the VKQueue handle
* of the context associated with the `context` parameter.
* \param r_queue_mutex: After calling this function the std::mutex referred
* by this parameter will contain the mutex of the context associated
* with the context parameter.
*/
void GHOST_GetVulkanHandles(GHOST_ContextHandle context,
void *r_instance,
void *r_physical_device,
void *r_device,
uint32_t *r_graphic_queue_family,
void *r_queue);
void *r_queue,
void **r_queue_mutex);
/**
* Set the pre and post callbacks for vulkan swap chain in the given context.

6
intern/ghost/GHOST_IContext.hh
View File

@@ -68,12 +68,16 @@ class GHOST_IContext {
* \param r_queue: After calling this function the VkQueue
* referenced by this parameter will contain the VKQueue handle
* of the context associated with the `context` parameter.
* \param r_queue_mutex: After calling this function the std::mutex referred
* by this parameter will contain the mutex of the context associated
* with the context parameter.
*/
virtual GHOST_TSuccess getVulkanHandles(void *r_instance,
void *r_physical_device,
void *r_device,
uint32_t *r_graphic_queue_family,
void *r_queue) = 0;
void *r_queue,
void **r_queue_mutex) = 0;
/**
* Acquire the current swap chain format.

5
intern/ghost/intern/GHOST_C-api.cc
View File

@@ -1256,11 +1256,12 @@ void GHOST_GetVulkanHandles(GHOST_ContextHandle contexthandle,
void *r_physical_device,
void *r_device,
uint32_t *r_graphic_queue_family,
void *r_queue)
void *r_queue,
void **r_queue_mutex)
{
GHOST_IContext *context = (GHOST_IContext *)contexthandle;
context->getVulkanHandles(
r_instance, r_physical_device, r_device, r_graphic_queue_family, r_queue);
r_instance, r_physical_device, r_device, r_graphic_queue_family, r_queue, r_queue_mutex);
}
void GHOST_SetVulkanSwapBuffersCallbacks(

6
intern/ghost/intern/GHOST_Context.hh
View File

@@ -154,6 +154,9 @@ class GHOST_Context : public GHOST_IContext {
* \param r_queue: After calling this function the VkQueue
* referenced by this parameter will contain the VKQueue handle
* of the context associated with the `context` parameter.
* \param r_queue_mutex: After calling this function the std::mutex referred
* by this parameter will contain the mutex of the context associated
* with the context parameter.
* \returns GHOST_kFailure when context isn't a Vulkan context.
* GHOST_kSuccess when the context is a Vulkan context and the
* handles have been set.
@@ -162,7 +165,8 @@ class GHOST_Context : public GHOST_IContext {
void * /*r_physical_device*/,
void * /*r_device*/,
uint32_t * /*r_graphic_queue_family*/,
void * /*r_queue*/) override
void * /*r_queue*/,
void ** /*r_queue_mutex*/) override
{
return GHOST_kFailure;
};

17
intern/ghost/intern/GHOST_ContextVK.cc
View File

@@ -27,6 +27,7 @@
#include <cstdio>
#include <cstring>
#include <iostream>
#include <mutex>
#include <optional>
#include <sstream>
@@ -136,6 +137,9 @@ class GHOST_DeviceVK {
int users = 0;
/** Mutex to externally synchronize access to queue. */
std::mutex queue_mutex;
public:
GHOST_DeviceVK(VkInstance vk_instance, VkPhysicalDevice vk_physical_device)
: instance(vk_instance), physical_device(vk_physical_device)
@@ -420,7 +424,7 @@ static GHOST_TSuccess ensure_vulkan_device(VkInstance vk_instance,
return GHOST_kFailure;
}
vulkan_device = std::make_optional<GHOST_DeviceVK>(vk_instance, best_physical_device);
vulkan_device.emplace(vk_instance, best_physical_device);
return GHOST_kSuccess;
}
@@ -564,7 +568,11 @@ GHOST_TSuccess GHOST_ContextVK::swapBuffers()
present_info.pImageIndices = &s_currentImage;
present_info.pResults = nullptr;
VkResult result = vkQueuePresentKHR(m_present_queue, &present_info);
VkResult result = VK_SUCCESS;
{
std::scoped_lock lock(vulkan_device->queue_mutex);
result = vkQueuePresentKHR(m_present_queue, &present_info);
}
if (result == VK_ERROR_OUT_OF_DATE_KHR || result == VK_SUBOPTIMAL_KHR) {
/* Swap-chain is out of date. Recreate swap-chain and skip this frame. */
destroySwapchain();
@@ -608,7 +616,8 @@ GHOST_TSuccess GHOST_ContextVK::getVulkanHandles(void *r_instance,
void *r_physical_device,
void *r_device,
uint32_t *r_graphic_queue_family,
void *r_queue)
void *r_queue,
void **r_queue_mutex)
{
*((VkInstance *)r_instance) = VK_NULL_HANDLE;
*((VkPhysicalDevice *)r_physical_device) = VK_NULL_HANDLE;
@@ -619,6 +628,8 @@ GHOST_TSuccess GHOST_ContextVK::getVulkanHandles(void *r_instance,
*((VkPhysicalDevice *)r_physical_device) = vulkan_device->physical_device;
*((VkDevice *)r_device) = vulkan_device->device;
*r_graphic_queue_family = vulkan_device->generic_queue_family;
std::mutex **queue_mutex = (std::mutex **)r_queue_mutex;
*queue_mutex = &vulkan_device->queue_mutex;
}
*((VkQueue *)r_queue) = m_graphic_queue;

3
intern/ghost/intern/GHOST_ContextVK.hh
View File

@@ -127,7 +127,8 @@ class GHOST_ContextVK : public GHOST_Context {
void *r_physical_device,
void *r_device,
uint32_t *r_graphic_queue_family,
void *r_queue) override;
void *r_queue,
void **r_queue_mutex) override;
GHOST_TSuccess getVulkanSwapChainFormat(GHOST_VulkanSwapChainData *r_swap_chain_data) override;

5
source/blender/gpu/vulkan/render_graph/vk_command_buffer_wrapper.cc
View File

@@ -94,7 +94,10 @@ void VKCommandBufferWrapper::submit_with_cpu_synchronization()
{
VKDevice &device = VKBackend::get().device;
vkResetFences(device.vk_handle(), 1, &vk_fence_);
vkQueueSubmit(device.queue_get(), 1, &vk_submit_info_, vk_fence_);
{
std::scoped_lock lock(device.queue_mutex_get());
vkQueueSubmit(device.queue_get(), 1, &vk_submit_info_, vk_fence_);
}
}
void VKCommandBufferWrapper::wait_for_cpu_synchronization()

5
source/blender/gpu/vulkan/vk_device.cc
View File

@@ -75,12 +75,15 @@ bool VKDevice::is_initialized() const
void VKDevice::init(void *ghost_context)
{
BLI_assert(!is_initialized());
void *queue_mutex = nullptr;
GHOST_GetVulkanHandles((GHOST_ContextHandle)ghost_context,
&vk_instance_,
&vk_physical_device_,
&vk_device_,
&vk_queue_family_,
&vk_queue_);
&vk_queue_,
&queue_mutex);
queue_mutex_ = static_cast<std::mutex *>(queue_mutex);
init_physical_device_properties();
init_physical_device_memory_properties();

5
source/blender/gpu/vulkan/vk_device.hh
View File

@@ -134,6 +134,7 @@ class VKDevice : public NonCopyable {
VkDevice vk_device_ = VK_NULL_HANDLE;
uint32_t vk_queue_family_ = 0;
VkQueue vk_queue_ = VK_NULL_HANDLE;
std::mutex *queue_mutex_ = nullptr;
VKSamplers samplers_;
VKDescriptorSetLayouts descriptor_set_layouts_;
@@ -233,6 +234,10 @@ class VKDevice : public NonCopyable {
{
return vk_queue_;
}
std::mutex &queue_mutex_get()
{
return *queue_mutex_;
}
const uint32_t queue_family_get() const
{