Commit 1dcd3cf4 authored by Dennis Tsiang's avatar Dennis Tsiang
Browse files

Remove use of pthread_cancel in the swapchain page flip thread



The current WSI layer may call pthread_cancel and kill the page flip
thread while it's still being used in Vulkan-API calls. This can cause the
layer to fail to clean up internal resources of the associated device,
leading to memory leaks. To resolve this, the layer now instead checks
if the page flip thread needs to end on every loop iteration. The
m_page_flip_semaphore has also been changed to a timed_semaphore to avoid
hanging indefinitely.

Change-Id: Iefbdd0d68e1f92fdf79bcebb7eaf33429fc61fd3
Signed-off-by: Dennis Tsiang's avatarDennis Tsiang <dennis.tsiang@arm.com>
parent 32623418
Pipeline #133482 passed with stage
in 1 minute and 4 seconds
......@@ -74,18 +74,34 @@ namespace wsi
* presented then the old one is marked as FREE and the free_image
* semaphore of the swapchain will be posted.
**/
__attribute__((noreturn)) void *page_flip_thread(void *ptr)
void *page_flip_thread(void *ptr)
{
auto *sc = static_cast<swapchain_base *>(ptr);
wsi::swapchain_image *sc_images = sc->m_swapchain_images;
VkResult vk_res = VK_SUCCESS;
uint64_t timeout = UINT64_MAX;
constexpr uint64_t SEMAPHORE_TIMEOUT = 250000000; /* 250 ms. */
while (true)
{
/* Check if this thread needs to be terminated. */
int res = pthread_mutex_lock(&sc->m_page_flip_thread_run_mutex);
assert(res == 0); /* only fails with programming error (EINVAL) */
bool quit = !sc->m_page_flip_thread_run;
res = pthread_mutex_unlock(&sc->m_page_flip_thread_run_mutex);
assert(res == 0); /* only fails with programming error (EPERM) */
if (quit)
{
break;
}
/* Waiting for the page_flip_semaphore which will be signalled once there is an
* image to display.*/
sem_wait(&sc->m_page_flip_semaphore);
if ((vk_res = sc->m_page_flip_semaphore.wait(SEMAPHORE_TIMEOUT)) == VK_TIMEOUT)
{
/* Image is not ready yet. */
continue;
}
assert(vk_res == VK_SUCCESS);
/* We want to present the oldest queued for present image from our present queue,
* which we can find at the sc->pending_buffer_pool.head index. */
......@@ -133,6 +149,7 @@ __attribute__((noreturn)) void *page_flip_thread(void *ptr)
sc->present_image(pending_index);
}
}
return nullptr;
}
void swapchain_base::unpresent_image(uint32_t presented_index)
......@@ -161,7 +178,10 @@ swapchain_base::swapchain_base(layer::device_private_data &dev_data, const VkAll
, m_ancestor(VK_NULL_HANDLE)
, m_device(VK_NULL_HANDLE)
, m_queue(VK_NULL_HANDLE)
, m_page_flip_thread_run(true)
{
int res = pthread_mutex_init(&m_page_flip_thread_run_mutex, NULL);
assert(res == 0);
}
VkResult swapchain_base::init(VkDevice device, const VkSwapchainCreateInfoKHR *swapchain_create_info)
......@@ -289,7 +309,11 @@ VkResult swapchain_base::init(VkDevice device, const VkSwapchainCreateInfoKHR *s
}
/* Setup semaphore for signaling pageflip thread */
res = sem_init(&m_page_flip_semaphore, 0, 0);
result = m_page_flip_semaphore.init(0);
if (result != VK_SUCCESS)
{
return result;
}
/* Only programming error can cause this to fail. */
assert(res == 0);
......@@ -377,11 +401,12 @@ void swapchain_base::teardown()
if (m_thread_sem_defined)
{
res = pthread_cancel(m_page_flip_thread);
if (res != 0)
{
WSI_PRINT_ERROR("pthread_cancel failed for page_flip_thread %lu with %d\n", m_page_flip_thread, res);
}
/* Tell flip thread to end. */
res = pthread_mutex_lock(&m_page_flip_thread_run_mutex);
assert(res == 0);
m_page_flip_thread_run = false;
res = pthread_mutex_unlock(&m_page_flip_thread_run_mutex);
assert(res == 0);
res = pthread_join(m_page_flip_thread, NULL);
if (res != 0)
......@@ -389,10 +414,10 @@ void swapchain_base::teardown()
WSI_PRINT_ERROR("pthread_join failed for page_flip_thread %lu with %d\n", m_page_flip_thread, res);
}
res = sem_destroy(&m_page_flip_semaphore);
res = pthread_mutex_destroy(&m_page_flip_thread_run_mutex);
if (res != 0)
{
WSI_PRINT_ERROR("sem_destroy failed for page_flip_semaphore with %d\n", errno);
WSI_PRINT_ERROR("pthread mutex destroyed for page flip thread %lu with %d\n", m_page_flip_thread, res);
}
res = sem_destroy(&m_start_present_semaphore);
......@@ -591,7 +616,7 @@ VkResult swapchain_base::queue_present(VkQueue queue, const VkPresentInfoKHR *pr
m_pending_buffer_pool.ring[m_pending_buffer_pool.tail] = image_index;
m_pending_buffer_pool.tail = (m_pending_buffer_pool.tail + 1) % m_pending_buffer_pool.size;
sem_post(&m_page_flip_semaphore);
m_page_flip_semaphore.post();
return VK_ERROR_OUT_OF_DATE_KHR;
}
......@@ -601,8 +626,7 @@ VkResult swapchain_base::queue_present(VkQueue queue, const VkPresentInfoKHR *pr
m_pending_buffer_pool.ring[m_pending_buffer_pool.tail] = image_index;
m_pending_buffer_pool.tail = (m_pending_buffer_pool.tail + 1) % m_pending_buffer_pool.size;
sem_post(&m_page_flip_semaphore);
m_page_flip_semaphore.post();
return VK_SUCCESS;
}
......
......@@ -145,6 +145,16 @@ protected:
*/
pthread_t m_page_flip_thread;
/**
* @brief Handle to the page flip thread mutex.
*/
pthread_mutex_t m_page_flip_thread_run_mutex;
/**
* @brief Whether the page flip thread has to continue running or terminate.
*/
bool m_page_flip_thread_run;
/**
* @brief In case we encounter threading or drm errors we need a way to
* notify the user of the failure. When this flag is false, acquire_next_image
......@@ -164,9 +174,9 @@ protected:
uint32_t size;
};
/**
* @brief A semaphore to be signalled once a page flip even occurs.
* @brief A semaphore to be signalled once a page flip event occurs.
*/
sem_t m_page_flip_semaphore;
util::timed_semaphore m_page_flip_semaphore;
/**
* @brief A semaphore to be signalled once the swapchain has one frame on screen.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment