Fixed #15 memory leak by introducing virtual function into all operation base classes to ensure the dependent class destructors are called

CMakeLists.txt

@@ -5,13 +5,15 @@ set(CMAKE_CXX_STANDARD 17)
 
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 
-set(CMAKE_CXX_FLAGS_DEBUG "-DDEBUG=1")
+set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DDEBUG=1")
-set(CMAKE_CXX_FLAGS_RELEASE "-DRELEASE=1")
+set(CMAKE_CXX_FLAGS_RELEASE "{CMAKE_CXX_FLAGS_RELEASE} -DRELEASE=1")
+
 
 set(CMAKE_VERBOSE_MAKEFILE on)
 
 option(KOMPUTE_OPT_BUILD_TESTS "Enable if you want to build tests" ON)
 option(KOMPUTE_OPT_BUILD_DOCS "Enable if you want to build documentation" ON)
+option(KOMPUTE_OPT_DEBUG_SYMBOLS "Enable if you want to build debug with symbols" 0)
 
 # Allow scripts to call main kompute Makefile
 function(kompute_make KOMPUTE_MAKE_TARGET)

single_include/kompute/Kompute.hpp

@@ -392,7 +392,7 @@ class OpBase
      * intended to destroy the resources in the parent class. This can be done
      * by passing the mFreeTensors=false.
      */
-    ~OpBase()
+    virtual ~OpBase()
     {
         SPDLOG_DEBUG("Kompute OpBase destructor started");
 
@@ -849,7 +849,7 @@ class OpAlgoBase : public OpBase
      * Default destructor, which is in charge of destroying the algorithm
      * components but does not destroy the underlying tensors
      */
-    ~OpAlgoBase();
+    virtual ~OpAlgoBase() override;
 
     /**
      * The init function is responsible for the initialisation of the algorithm
@@ -1057,12 +1057,19 @@ std::vector<char> OpAlgoBase<tX, tY, tZ>::fetchSpirvBinaryData()
     std::ifstream fileStream(this->mOptSpirvBinPath,
                              std::ios::binary | std::ios::in | std::ios::ate);
 
+    if (!fileStream.good()) {
+        throw std::runtime_error("Error reading file: " + this->mOptSpirvBinPath);
+    }
+
     size_t shaderFileSize = fileStream.tellg();
     fileStream.seekg(0, std::ios::beg);
     char* shaderDataRaw = new char[shaderFileSize];
     fileStream.read(shaderDataRaw, shaderFileSize);
     fileStream.close();
 
+    SPDLOG_WARN(
+      "Kompute OpAlgoBase fetched {} bytes", shaderFileSize);
+
     return std::vector<char>(shaderDataRaw,
                              shaderDataRaw + shaderFileSize);
 }
@@ -1112,7 +1119,7 @@ class OpAlgoLhsRhsOut : public OpAlgoBase<tX, tY, tZ>
      * Default destructor, which is in charge of destroying the algorithm
      * components but does not destroy the underlying tensors
      */
-    ~OpAlgoLhsRhsOut();
+    virtual ~OpAlgoLhsRhsOut() override;
 
     /**
      * The init function is responsible for ensuring that all of the tensors
@@ -1357,7 +1364,7 @@ class OpMult : public OpAlgoBase<tX, tY, tZ>
      * Default destructor, which is in charge of destroying the algorithm
      * components but does not destroy the underlying tensors
      */
-    ~OpMult() {
+    ~OpMult() override {
         SPDLOG_DEBUG("Kompute OpMult destructor started");
     }
 
@@ -1396,7 +1403,7 @@ class OpCreateTensor : public OpBase
      * Default destructor which in this case expects the parent class to free
      * the tensors
      */
-    ~OpCreateTensor();
+    ~OpCreateTensor() override;
 
     /**
      * In charge of initialising the primary Tensor as well as the staging

src/Algorithm.cpp

@@ -27,6 +27,62 @@ Algorithm::~Algorithm()
           "Kompute Algorithm destructor reached with null Device pointer");
         return;
     }
+
+    if (this->mFreePipeline) {
+        SPDLOG_DEBUG("Kompute Algorithm Destroying pipeline");
+        if (!this->mPipeline) {
+            SPDLOG_ERROR("Kompute Algorithm Error requested to destroy pipeline but it is null");
+        }
+        this->mDevice->destroy(*this->mPipeline);
+    }
+
+    if (this->mFreePipelineCache) {
+        SPDLOG_DEBUG("Kompute Algorithm Destroying pipeline cache");
+        if (!this->mPipelineCache) {
+            SPDLOG_ERROR("Kompute Algorithm Error requested to destroy pipeline cache but it is null");
+        }
+        this->mDevice->destroy(*this->mPipelineCache);
+    }
+
+    if (this->mFreePipelineLayout) {
+        SPDLOG_DEBUG("Kompute Algorithm Destroying pipeline layout");
+        if (!this->mPipelineLayout) {
+            SPDLOG_ERROR("Kompute Algorithm Error requested to destroy pipeline layout but it is null");
+        }
+        this->mDevice->destroy(*this->mPipelineLayout);
+    }
+
+    if (this->mFreeShaderModule) {
+        SPDLOG_DEBUG("Kompute Algorithm Destroying shader module");
+        if (!this->mShaderModule) {
+            SPDLOG_ERROR("Kompute Algorithm Error requested to destroy shader module but it is null");
+        }
+        this->mDevice->destroy(*this->mShaderModule);
+    }
+
+    if (this->mFreeDescriptorSet) {
+        SPDLOG_DEBUG("Kompute Algorithm Freeing Descriptor Set");
+        if (!this->mDescriptorSet) {
+            SPDLOG_ERROR("Kompute Algorithm Error requested to free descriptor set");
+        }
+        this->mDevice->freeDescriptorSets(*this->mDescriptorPool, 1, this->mDescriptorSet.get());
+    }
+
+    if (this->mFreeDescriptorSetLayout) {
+        SPDLOG_DEBUG("Kompute Algorithm Destroying Descriptor Set Layout");
+        if (!this->mDescriptorSetLayout) {
+            SPDLOG_ERROR("Kompute Algorithm Error requested to destroy descriptor set layout but it is null");
+        }
+        this->mDevice->destroy(*this->mDescriptorSetLayout);
+    }
+
+    if (this->mFreeDescriptorPool) {
+        SPDLOG_DEBUG("Kompute Algorithm Destroying Descriptor Pool");
+        if (!this->mDescriptorPool) {
+            SPDLOG_ERROR("Kompute Algorithm Error requested to destroy descriptor pool but it is null");
+        }
+        this->mDevice->destroy(*this->mDescriptorPool);
+    }
 }
 
 void
@@ -99,6 +155,7 @@ Algorithm::createParameters(std::vector<std::shared_ptr<Tensor>>& tensorParams)
     this->mDescriptorSetLayout = std::make_shared<vk::DescriptorSetLayout>();
     this->mDevice->createDescriptorSetLayout(
       &descriptorSetLayoutInfo, nullptr, this->mDescriptorSetLayout.get());
+    this->mFreeDescriptorSetLayout = true;
 
     vk::DescriptorSetAllocateInfo descriptorSetAllocateInfo(
       *this->mDescriptorPool,
@@ -109,6 +166,7 @@ Algorithm::createParameters(std::vector<std::shared_ptr<Tensor>>& tensorParams)
     this->mDescriptorSet = std::make_shared<vk::DescriptorSet>();
     this->mDevice->allocateDescriptorSets(&descriptorSetAllocateInfo,
                                           this->mDescriptorSet.get());
+    this->mFreeDescriptorSet = true;
 
     // TODO: Explore design exposing the destination array element
     for (size_t i = 0; i < tensorParams.size(); i++) {
@@ -153,6 +211,7 @@ Algorithm::createShaderModule(const std::vector<char>& shaderFileData)
     this->mShaderModule = std::make_shared<vk::ShaderModule>();
     this->mDevice->createShaderModule(
       &shaderModuleInfo, nullptr, this->mShaderModule.get());
+    this->mFreeShaderModule = true;
 
     SPDLOG_DEBUG("Kompute Algorithm create shader module success");
 }
@@ -171,6 +230,7 @@ Algorithm::createPipeline(std::vector<uint32_t> specializationData)
     this->mPipelineLayout = std::make_shared<vk::PipelineLayout>();
     this->mDevice->createPipelineLayout(
       &pipelineLayoutInfo, nullptr, this->mPipelineLayout.get());
+    this->mFreePipelineLayout = true;
 
     std::vector<vk::SpecializationMapEntry> specializationEntries;
 
@@ -203,15 +263,16 @@ Algorithm::createPipeline(std::vector<uint32_t> specializationData)
                                                0);
 
     // TODO: Confirm what the best structure is with pipeline cache
-    this->mFreePipelineCache = true;
     vk::PipelineCacheCreateInfo pipelineCacheInfo =
       vk::PipelineCacheCreateInfo();
     this->mPipelineCache = std::make_shared<vk::PipelineCache>();
     this->mDevice->createPipelineCache(
       &pipelineCacheInfo, nullptr, this->mPipelineCache.get());
+    this->mFreePipelineCache = true;
 
     vk::ResultValue<vk::Pipeline> pipelineResult =
       this->mDevice->createComputePipeline(*this->mPipelineCache, pipelineInfo);
+    this->mFreePipeline = true;
 
     if (pipelineResult.result != vk::Result::eSuccess) {
         throw std::runtime_error("Failed to create pipeline result: " +

src/include/kompute/operations/OpAlgoBase.hpp

@@ -61,7 +61,7 @@ class OpAlgoBase : public OpBase
      * Default destructor, which is in charge of destroying the algorithm
      * components but does not destroy the underlying tensors
      */
-    ~OpAlgoBase();
+    virtual ~OpAlgoBase() override;
 
     /**
      * The init function is responsible for the initialisation of the algorithm
@@ -269,12 +269,19 @@ std::vector<char> OpAlgoBase<tX, tY, tZ>::fetchSpirvBinaryData()
     std::ifstream fileStream(this->mOptSpirvBinPath,
                              std::ios::binary | std::ios::in | std::ios::ate);
 
+    if (!fileStream.good()) {
+        throw std::runtime_error("Error reading file: " + this->mOptSpirvBinPath);
+    }
+
     size_t shaderFileSize = fileStream.tellg();
     fileStream.seekg(0, std::ios::beg);
     char* shaderDataRaw = new char[shaderFileSize];
     fileStream.read(shaderDataRaw, shaderFileSize);
     fileStream.close();
 
+    SPDLOG_WARN(
+      "Kompute OpAlgoBase fetched {} bytes", shaderFileSize);
+
     return std::vector<char>(shaderDataRaw,
                              shaderDataRaw + shaderFileSize);
 }

src/include/kompute/operations/OpAlgoLhsRhsOut.hpp

@@ -48,7 +48,7 @@ class OpAlgoLhsRhsOut : public OpAlgoBase<tX, tY, tZ>
      * Default destructor, which is in charge of destroying the algorithm
      * components but does not destroy the underlying tensors
      */
-    ~OpAlgoLhsRhsOut();
+    virtual ~OpAlgoLhsRhsOut() override;
 
     /**
      * The init function is responsible for ensuring that all of the tensors

src/include/kompute/operations/OpBase.hpp

@@ -54,7 +54,7 @@ class OpBase
      * intended to destroy the resources in the parent class. This can be done
      * by passing the mFreeTensors=false.
      */
-    ~OpBase()
+    virtual ~OpBase()
     {
         SPDLOG_DEBUG("Kompute OpBase destructor started");
 

src/include/kompute/operations/OpCreateTensor.hpp

@@ -37,7 +37,7 @@ class OpCreateTensor : public OpBase
      * Default destructor which in this case expects the parent class to free
      * the tensors
      */
-    ~OpCreateTensor();
+    ~OpCreateTensor() override;
 
     /**
      * In charge of initialising the primary Tensor as well as the staging

src/include/kompute/operations/OpMult.hpp

@@ -84,7 +84,7 @@ class OpMult : public OpAlgoBase<tX, tY, tZ>
      * Default destructor, which is in charge of destroying the algorithm
      * components but does not destroy the underlying tensors
      */
-    ~OpMult() {
+    ~OpMult() override {
         SPDLOG_DEBUG("Kompute OpMult destructor started");
     }
 

test/TestManager.cpp

@@ -9,35 +9,37 @@ TEST_CASE("End to end OpMult Flow should execute correctly from manager") {
     spdlog::info("TEST CASE STARTING");
 
     spdlog::info("Creating manager");
-    kp::Manager mgr;
+    {
+        kp::Manager mgr;
 
-    spdlog::info("Creating first tensor");
+        spdlog::info("Creating first tensor");
-    std::shared_ptr<kp::Tensor> tensorLHS{ new kp::Tensor( { 0, 1, 2 }) };
+        std::shared_ptr<kp::Tensor> tensorLHS{ new kp::Tensor({ 0, 1, 2 }) };
-    mgr.evalOp<kp::OpCreateTensor>({ tensorLHS });
+        mgr.evalOp<kp::OpCreateTensor>({ tensorLHS });
 
-    spdlog::info("Creating second tensor");
+        spdlog::info("Creating second tensor");
-    std::shared_ptr<kp::Tensor> tensorRHS{ new kp::Tensor(
+        std::shared_ptr<kp::Tensor> tensorRHS{ new kp::Tensor(
-      { 2, 4, 6 }) };
+          { 2, 4, 6 }) };
-    mgr.evalOp<kp::OpCreateTensor>({ tensorRHS });
+        mgr.evalOp<kp::OpCreateTensor>({ tensorRHS });
 
-    // TODO: Add capabilities for just output tensor types
+        // TODO: Add capabilities for just output tensor types
-    spdlog::info("Creating output tensor");
+        spdlog::info("Creating output tensor");
-    std::shared_ptr<kp::Tensor> tensorOutput{ new kp::Tensor(
+        std::shared_ptr<kp::Tensor> tensorOutput{ new kp::Tensor(
-      { 0, 0, 0 }) };
+          { 0, 0, 0 }) };
-    mgr.evalOp<kp::OpCreateTensor>({ tensorOutput });
+        mgr.evalOp<kp::OpCreateTensor>({ tensorOutput });
 
-    spdlog::info("OpCreateTensor success for tensors");
+        spdlog::info("OpCreateTensor success for tensors");
-    spdlog::info("Tensor one: {}", tensorLHS->data());
+        spdlog::info("Tensor one: {}", tensorLHS->data());
-    spdlog::info("Tensor two: {}", tensorRHS->data());
+        spdlog::info("Tensor two: {}", tensorRHS->data());
-    spdlog::info("Tensor output: {}", tensorOutput->data());
+        spdlog::info("Tensor output: {}", tensorOutput->data());
 
-    spdlog::info("Calling op mult");
+        spdlog::info("Calling op mult");
-    mgr.evalOp<kp::OpMult<>>({ tensorLHS, tensorRHS, tensorOutput });
+        mgr.evalOp<kp::OpMult<>>({ tensorLHS, tensorRHS, tensorOutput });
 
-    spdlog::info("OpMult call success");
+        spdlog::info("OpMult call success");
-    spdlog::info("Tensor output: {}", tensorOutput->data());
+        spdlog::info("Tensor output: {}", tensorOutput->data());
 
-    REQUIRE(tensorOutput->data() == std::vector<uint32_t>{0, 4, 12});
+        REQUIRE(tensorOutput->data() == std::vector<uint32_t>{0, 4, 12});
+    }
 
     spdlog::info("Called manager eval success END PROGRAM");
 }