Fix CommandPool memory management
Ever since vk::destroy calls the object destructor, we can have std
objects in our vulkan objects, rather than std object pointers, which
allows us to allocate them using the provided allocator, rather than
having to use DEVICE_MEMORY.
I'm unsure whether there was an actual memory leak here or if ASAN
was detecting something that wasn't there, but this cl cleans up std
object allocation and fixes whatever ASAN was detecting. With this
change, the fuzzer run now completes without issue.
Bug: chromium:1107112
Change-Id: Ib78451639131eb518e2bf8d63ba70b930aba2216
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/48328
Presubmit-Ready: Alexis Hétu <sugoi@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/Vulkan/VkCommandBuffer.cpp b/src/Vulkan/VkCommandBuffer.cpp
index cff9139..f178d35 100644
--- a/src/Vulkan/VkCommandBuffer.cpp
+++ b/src/Vulkan/VkCommandBuffer.cpp
@@ -36,15 +36,6 @@
#include <cstring>
-class vk::CommandBuffer::Command
-{
-public:
- // FIXME (b/119421344): change the commandBuffer argument to a CommandBuffer state
- virtual void play(vk::CommandBuffer::ExecutionState &executionState) = 0;
- virtual std::string description() = 0;
- virtual ~Command() {}
-};
-
namespace {
class CmdBeginRenderPass : public vk::CommandBuffer::Command
@@ -1316,19 +1307,16 @@
: device(device)
, level(pLevel)
{
- // FIXME (b/119409619): replace this vector by an allocator so we can control all memory allocations
- commands = new std::vector<std::unique_ptr<Command>>();
}
void CommandBuffer::destroy(const VkAllocationCallbacks *pAllocator)
{
- delete commands;
}
void CommandBuffer::resetState()
{
// FIXME (b/119409619): replace this vector by an allocator so we can control all memory allocations
- commands->clear();
+ commands.clear();
state = INITIAL;
}
@@ -1377,7 +1365,7 @@
if(debuggerContext)
{
std::string source;
- for(auto &command : *commands)
+ for(auto &command : commands)
{
source += command->description() + "\n";
}
@@ -1401,7 +1389,7 @@
void CommandBuffer::addCommand(Args &&... args)
{
// FIXME (b/119409619): use an allocator here so we can control all memory allocations
- commands->push_back(std::make_unique<T>(std::forward<Args>(args)...));
+ commands.push_back(std::make_unique<T>(std::forward<Args>(args)...));
}
void CommandBuffer::beginRenderPass(RenderPass *renderPass, Framebuffer *framebuffer, VkRect2D renderArea,
@@ -1826,7 +1814,7 @@
int line = 1;
#endif // ENABLE_VK_DEBUGGER
- for(auto &command : *commands)
+ for(auto &command : commands)
{
#ifdef ENABLE_VK_DEBUGGER
if(debuggerThread)
@@ -1846,7 +1834,7 @@
void CommandBuffer::submitSecondary(CommandBuffer::ExecutionState &executionState) const
{
- for(auto &command : *commands)
+ for(auto &command : commands)
{
command->play(executionState);
}
diff --git a/src/Vulkan/VkCommandBuffer.hpp b/src/Vulkan/VkCommandBuffer.hpp
index 7b6a61e..aeaba58 100644
--- a/src/Vulkan/VkCommandBuffer.hpp
+++ b/src/Vulkan/VkCommandBuffer.hpp
@@ -192,7 +192,13 @@
void submit(CommandBuffer::ExecutionState &executionState);
void submitSecondary(CommandBuffer::ExecutionState &executionState) const;
- class Command;
+ class Command
+ {
+ public:
+ virtual void play(ExecutionState &executionState) = 0;
+ virtual std::string description() = 0;
+ virtual ~Command() {}
+ };
private:
void resetState();
@@ -213,7 +219,7 @@
VkCommandBufferLevel level = VK_COMMAND_BUFFER_LEVEL_PRIMARY;
// FIXME (b/119409619): replace this vector by an allocator so we can control all memory allocations
- std::vector<std::unique_ptr<Command>> *commands;
+ std::vector<std::unique_ptr<Command>> commands;
#ifdef ENABLE_VK_DEBUGGER
std::shared_ptr<vk::dbg::File> debuggerFile;
diff --git a/src/Vulkan/VkCommandPool.cpp b/src/Vulkan/VkCommandPool.cpp
index 96a833f..a7d6fe2 100644
--- a/src/Vulkan/VkCommandPool.cpp
+++ b/src/Vulkan/VkCommandPool.cpp
@@ -22,23 +22,15 @@
CommandPool::CommandPool(const VkCommandPoolCreateInfo *pCreateInfo, void *mem)
{
- // FIXME (b/119409619): use an allocator here so we can control all memory allocations
- void *deviceMemory = vk::allocate(sizeof(std::set<VkCommandBuffer>), REQUIRED_MEMORY_ALIGNMENT,
- DEVICE_MEMORY, GetAllocationScope());
- ASSERT(deviceMemory);
- commandBuffers = new(deviceMemory) std::set<VkCommandBuffer>();
}
void CommandPool::destroy(const VkAllocationCallbacks *pAllocator)
{
// Free command Buffers allocated in allocateCommandBuffers
- for(auto commandBuffer : *commandBuffers)
+ for(auto commandBuffer : commandBuffers)
{
vk::destroy(commandBuffer, DEVICE_MEMORY);
}
-
- // FIXME (b/119409619): use an allocator here so we can control all memory allocations
- vk::deallocate(commandBuffers, DEVICE_MEMORY);
}
size_t CommandPool::ComputeRequiredAllocationSize(const VkCommandPoolCreateInfo *pCreateInfo)
@@ -73,7 +65,7 @@
}
}
- commandBuffers->insert(pCommandBuffers, pCommandBuffers + commandBufferCount);
+ commandBuffers.insert(pCommandBuffers, pCommandBuffers + commandBufferCount);
return VK_SUCCESS;
}
@@ -82,7 +74,7 @@
{
for(uint32_t i = 0; i < commandBufferCount; ++i)
{
- commandBuffers->erase(pCommandBuffers[i]);
+ commandBuffers.erase(pCommandBuffers[i]);
vk::destroy(pCommandBuffers[i], DEVICE_MEMORY);
}
}
@@ -92,17 +84,11 @@
// According the Vulkan 1.1 spec:
// "All command buffers that have been allocated from
// the command pool are put in the initial state."
- for(auto commandBuffer : *commandBuffers)
+ for(auto commandBuffer : commandBuffers)
{
vk::Cast(commandBuffer)->reset(flags);
}
- // According the Vulkan 1.1 spec:
- // "Resetting a command pool recycles all of the
- // resources from all of the command buffers allocated
- // from the command pool back to the command pool."
- commandBuffers->clear();
-
return VK_SUCCESS;
}
diff --git a/src/Vulkan/VkCommandPool.hpp b/src/Vulkan/VkCommandPool.hpp
index 0aec423..bdad81b 100644
--- a/src/Vulkan/VkCommandPool.hpp
+++ b/src/Vulkan/VkCommandPool.hpp
@@ -37,7 +37,7 @@
void trim(VkCommandPoolTrimFlags flags);
private:
- std::set<VkCommandBuffer> *commandBuffers;
+ std::set<VkCommandBuffer> commandBuffers;
};
static inline CommandPool *Cast(VkCommandPool object)
