From 86fa15a8a2ac5e1fb2d8d9aa64a968e501e1d77f Mon Sep 17 00:00:00 2001 From: Normunds Rieksts Date: Mon, 8 Nov 2021 17:53:12 +0000 Subject: [PATCH] Fix OOM issue Fixes an issue where there was a chance that an exception would be thrown when out of memory rather than returning an appropriate VkResult code. Additionally introduces a noncopyable utility class that can be used to mark classes that should not have copy semantics Change-Id: I1f84dc9bb1ea96db2a88a90d56adbee78b17c5e3 Signed-off-by: Normunds Rieksts --- util/custom_allocator.hpp | 4 +++- util/extension_list.hpp | 8 +++----- util/file_descriptor.hpp | 7 +++---- util/helpers.hpp | 11 +++++++++++ util/optional.hpp | 6 ++---- util/timed_semaphore.hpp | 7 ++----- util/unordered_map.hpp | 6 ++---- util/unordered_set.hpp | 6 ++---- wsi/wayland/swapchain.cpp | 12 ++++++------ wsi/wayland/swapchain.hpp | 4 ++-- 10 files changed, 36 insertions(+), 35 deletions(-) diff --git a/util/custom_allocator.hpp b/util/custom_allocator.hpp index 829c798..070c82b 100644 --- a/util/custom_allocator.hpp +++ b/util/custom_allocator.hpp @@ -30,6 +30,8 @@ #include +#include "helpers.hpp" + #pragma once namespace util @@ -280,7 +282,7 @@ util::unique_ptr allocator::make_unique(Args &&...args) const noexcept * vector. */ template -class vector : public std::vector> +class vector : public std::vector>, private noncopyable { public: using base = std::vector>; diff --git a/util/extension_list.hpp b/util/extension_list.hpp index 366cfd2..a71888e 100644 --- a/util/extension_list.hpp +++ b/util/extension_list.hpp @@ -23,7 +23,8 @@ */ #pragma once -#include "util/custom_allocator.hpp" +#include "custom_allocator.hpp" +#include "helpers.hpp" #include #include @@ -38,14 +39,11 @@ namespace util * * @note This class does not store the extension versions. */ -class extension_list +class extension_list : private noncopyable { public: extension_list(const util::allocator& allocator); - extension_list(const extension_list &rhs) = delete; - const extension_list &operator=(const extension_list &rhs) = delete; - /** * @brief Get the allocator used to manage the memory of this object. */ diff --git a/util/file_descriptor.hpp b/util/file_descriptor.hpp index f762f99..664848f 100644 --- a/util/file_descriptor.hpp +++ b/util/file_descriptor.hpp @@ -33,13 +33,15 @@ #include #include +#include "helpers.hpp" + namespace util { /** * Manages a POSIX file descriptor. */ -class fd_owner +class fd_owner : private noncopyable { public: @@ -49,9 +51,6 @@ public: { } - fd_owner(const fd_owner &) = delete; - fd_owner &operator=(const fd_owner &) = delete; - fd_owner(fd_owner &&rhs) { *this = std::move(rhs); diff --git a/util/helpers.hpp b/util/helpers.hpp index 1dff400..9b5932e 100644 --- a/util/helpers.hpp +++ b/util/helpers.hpp @@ -44,4 +44,15 @@ const T *find_extension(VkStructureType sType, const void *pNext) } return reinterpret_cast(entry); } + +class noncopyable +{ +protected: + noncopyable() = default; + ~noncopyable() = default; + +private: + noncopyable(const noncopyable &) = delete; + noncopyable& operator=(const noncopyable &) = delete; +}; } // namespace util diff --git a/util/optional.hpp b/util/optional.hpp index cf3fa52..469dbf1 100644 --- a/util/optional.hpp +++ b/util/optional.hpp @@ -24,18 +24,16 @@ #pragma once #include +#include "helpers.hpp" namespace util { template -class optional +class optional : private noncopyable { public: using value_type = T; - optional(const optional &) = delete; - optional &operator=(const optional &) = delete; - /** * @brief Construct an empty optional object. */ diff --git a/util/timed_semaphore.hpp b/util/timed_semaphore.hpp index 5988550..9f5b867 100644 --- a/util/timed_semaphore.hpp +++ b/util/timed_semaphore.hpp @@ -45,6 +45,7 @@ extern "C" } #include +#include "helpers.hpp" namespace util { @@ -61,13 +62,9 @@ namespace util * * This code does not use the C++ standard library to avoid exceptions. */ -class timed_semaphore +class timed_semaphore : private noncopyable { public: - /* copying not implemented */ - timed_semaphore &operator=(const timed_semaphore &) = delete; - timed_semaphore(const timed_semaphore &) = delete; - ~timed_semaphore(); timed_semaphore() : initialized(false){}; diff --git a/util/unordered_map.hpp b/util/unordered_map.hpp index 90c8c9a..daf64c2 100644 --- a/util/unordered_map.hpp +++ b/util/unordered_map.hpp @@ -26,6 +26,7 @@ #include #include "custom_allocator.hpp" #include "optional.hpp" +#include "helpers.hpp" namespace util { @@ -39,7 +40,7 @@ template , typename Comparator = std::equal_to, typename Allocator = util::custom_allocator>> -class unordered_map : public std::unordered_map +class unordered_map : public std::unordered_map, private noncopyable { using base = std::unordered_map; using size_type = typename base::size_type; @@ -52,9 +53,6 @@ public: Value &operator[](const Key &key) = delete; Value &operator[](Key &&key) = delete; - unordered_map(const unordered_map &) = delete; - unordered_map &operator=(const unordered_map &) = delete; - void insert() = delete; void emplace() = delete; void emplace_hint() = delete; diff --git a/util/unordered_set.hpp b/util/unordered_set.hpp index 3fa6035..c07ffa5 100644 --- a/util/unordered_set.hpp +++ b/util/unordered_set.hpp @@ -26,6 +26,7 @@ #include #include "custom_allocator.hpp" #include "optional.hpp" +#include "helpers.hpp" namespace util { @@ -39,7 +40,7 @@ template , typename Comparator = std::equal_to, typename Allocator = util::custom_allocator> -class unordered_set : public std::unordered_set +class unordered_set : public std::unordered_set, private noncopyable { using value_type = Key; using base = std::unordered_set; @@ -50,9 +51,6 @@ public: /** * Delete all member functions that can cause allocation failure by throwing std::bad_alloc. */ - unordered_set(const unordered_set &) = delete; - unordered_set &operator=(const unordered_set &) = delete; - void insert() = delete; void emplace() = delete; void emplace_hint() = delete; diff --git a/wsi/wayland/swapchain.cpp b/wsi/wayland/swapchain.cpp index 8f33da3..5b56cc4 100644 --- a/wsi/wayland/swapchain.cpp +++ b/wsi/wayland/swapchain.cpp @@ -399,8 +399,8 @@ VkResult swapchain::create_aliased_image_handle(const VkImageCreateInfo *image_c get_allocation_callbacks(), image); } -VkResult swapchain::allocate_wsialloc(VkImageCreateInfo &image_create_info, wayland_image_data *image_data, - util::vector importable_formats, +VkResult swapchain::allocate_wsialloc(VkImageCreateInfo &image_create_info, wayland_image_data &image_data, + util::vector &importable_formats, wsialloc_format *allocated_format) { bool is_protected_memory = (image_create_info.flags & VK_IMAGE_CREATE_PROTECTED_BIT) != 0; @@ -408,8 +408,8 @@ VkResult swapchain::allocate_wsialloc(VkImageCreateInfo &image_create_info, wayl wsialloc_allocate_info alloc_info = { importable_formats.data(), static_cast(importable_formats.size()), image_create_info.extent.width, image_create_info.extent.height, allocation_flags }; - const auto res = wsialloc_alloc(m_wsi_allocator, &alloc_info, allocated_format, image_data->stride, - image_data->buffer_fd, image_data->offset); + const auto res = wsialloc_alloc(m_wsi_allocator, &alloc_info, allocated_format, image_data.stride, + image_data.buffer_fd, image_data.offset); if (res != WSIALLOC_ERROR_NONE) { WSI_LOG_ERROR("Failed allocation of DMA Buffer. WSI error: %d", static_cast(res)); @@ -443,7 +443,7 @@ VkResult swapchain::allocate_image(VkImageCreateInfo &image_create_info, wayland { return VK_ERROR_OUT_OF_HOST_MEMORY; } - VkResult result = allocate_wsialloc(m_image_create_info, image_data, importable_formats, &m_allocated_format); + VkResult result = allocate_wsialloc(m_image_create_info, *image_data, importable_formats, &m_allocated_format); if (result != VK_SUCCESS) { return result; @@ -474,7 +474,7 @@ VkResult swapchain::allocate_image(VkImageCreateInfo &image_create_info, wayland } wsialloc_format allocated_format = { 0 }; - result = allocate_wsialloc(image_create_info, image_data, importable_formats, &allocated_format); + result = allocate_wsialloc(image_create_info, *image_data, importable_formats, &allocated_format); if (result != VK_SUCCESS) { return result; diff --git a/wsi/wayland/swapchain.hpp b/wsi/wayland/swapchain.hpp index d9dc3b1..c5fcfd2 100644 --- a/wsi/wayland/swapchain.hpp +++ b/wsi/wayland/swapchain.hpp @@ -163,8 +163,8 @@ private: struct wayland_image_data; VkResult allocate_image(VkImageCreateInfo &image_create_info, wayland_image_data *image_data, VkImage *image); - VkResult allocate_wsialloc(VkImageCreateInfo &image_create_info, wayland_image_data *image_data, - util::vector importable_formats, wsialloc_format *allocated_format); + VkResult allocate_wsialloc(VkImageCreateInfo &image_create_info, wayland_image_data &image_data, + util::vector &importable_formats, wsialloc_format *allocated_format); VkResult internal_bind_swapchain_image(VkDevice &device, wayland_image_data *swapchain_image, const VkImage &image);