From effedbb3f04e585c42cdab84fa6960ab2c3a3b9c Mon Sep 17 00:00:00 2001
From: NAUD Maxence <maxence.naud@cea.fr>
Date: Wed, 9 Aug 2023 13:40:58 +0000
Subject: [PATCH] [Bug] Solve memory leak for complex type parameters
serialization
Providing a vector as a Parameter for a GenericOperator caused a memory leak.
Solved using malloc instead of pointers to vector's data
---
include/aidge/utils/CParameter.hpp | 43 +++++++++++++++++-------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/include/aidge/utils/CParameter.hpp b/include/aidge/utils/CParameter.hpp
index c7d0ea23d..a8f18c03f 100644
--- a/include/aidge/utils/CParameter.hpp
+++ b/include/aidge/utils/CParameter.hpp
@@ -16,6 +16,7 @@
#include <map>
#include <vector>
#include <numeric>
+#include <cstddef>
namespace Aidge {
@@ -45,9 +46,9 @@ public:
{
assert(m_Params.find(i_ParamName) != m_Params.end());
assert(m_Types.find(i_ParamName) != m_Types.end());
- assert(m_Params.at(i_ParamName) <= m_OffSet);
+ assert(m_Params.at(i_ParamName) <= m_Size);
assert(typeid(T).name() == m_Types.at(i_ParamName));
- return *reinterpret_cast<T *>(m_BeginBuffer + m_Params.at(i_ParamName));
+ return *reinterpret_cast<T *>(m_Buffer + m_Params.at(i_ParamName));
}
///\brief Add a parameter value, identified by its name
@@ -59,12 +60,21 @@ public:
/// internal buffer in a new location (previous value is still in memory at its previous location)
template<class T> void Add(std::string const &i_ParamName, T const &i_Value)
{
- m_Buffer.resize(m_Buffer.size() + (sizeof(T) / sizeof(uint8_t)));
- m_BeginBuffer = m_Buffer.data(); // Update buffer ptr in case of memory reordering
- *reinterpret_cast<T *>(m_BeginBuffer + m_OffSet)
- = i_Value; // Black-magic used to add anytype into the vector
- m_Params[i_ParamName] = m_OffSet; // Copy pointer offset
- m_OffSet += sizeof(T); // Increment offset
+ const std::size_t addedSize = sizeof(T) / sizeof(std::uint8_t);
+ std::uint8_t *tmp = m_Buffer;
+ std::uint8_t *m_NewBuffer = static_cast<std::uint8_t *>(std::malloc((m_Size + addedSize)*sizeof(std::uint8_t)));
+
+ for (std::size_t i = 0; i < m_Size; ++i) {
+ m_NewBuffer[i] = m_Buffer[i];
+ }
+ free(tmp);
+ for (std::size_t i = 0; i < addedSize; ++i) {
+ m_NewBuffer[m_Size+i] = *(reinterpret_cast<const std::uint8_t *>(&i_Value) + i);
+ }
+ m_Buffer = m_NewBuffer;
+
+ m_Params[i_ParamName] = m_Size; // Copy pointer offset
+ m_Size += addedSize; // Increment offset
m_Types[i_ParamName] = typeid(i_Value).name();
}
@@ -80,10 +90,14 @@ public:
}
- ~CParameter() = default;
+ ~CParameter() {
+ free(m_Buffer);
+ }
private:
- // Note for Cyril: of course storing offset and not address! Good idea
+ /// @brief Number of elements in m_Buffer
+ std::size_t m_Size = 0;
+
std::map<std::string, std::size_t> m_Params; // { Param name : offset }
///\brief Map to check type error
@@ -95,16 +109,9 @@ private:
std::map<std::string, std::string> m_Types;
///\brief All parameters values concatenated in raw binary form.
- std::vector<uint8_t> m_Buffer = {};
-
- ///\brief Starting address of the buffer
- uint8_t *m_BeginBuffer = m_Buffer.data();
-
- ///\brief Offset, in number of uint8_t, of the next parameter to write
- std::size_t m_OffSet = 0;
+ std::uint8_t *m_Buffer = static_cast<std::uint8_t *>(std::malloc(0));
};
}
-
#endif /* __AIDGE_CPARAMETER_H__ */
--
GitLab