Skip to content
Snippets Groups Projects
Commit 42a7741a authored by Maxence Naud's avatar Maxence Naud
Browse files

[Fix] Memory leak due to containers serialization

- [Add] minimal std::any implementation compatible with C++14
GenericOperator parameters are accessed by reference
parent 75a0d5a6
No related branches found
No related tags found
1 merge request!15Remove CParameter memory leak
Pipeline #30880 failed
......@@ -62,7 +62,12 @@ class GenericOperator_Op
* @return template<class T> The parameter.
*/
template <class T>
T getParameter(std::string const &key) const {
const T& getParameter(std::string const &key) const {
return mParams.Get<const T>(key);
}
template <class T>
T& getParameter(std::string const &key) {
return mParams.Get<T>(key);
}
......@@ -75,8 +80,8 @@ class GenericOperator_Op
/// internal buffer in a new location (previous value is still in memory at
/// its previous location)
template <class T>
void addParameter(std::string const &key, T const &value) {
mParams.Add<T>(key, value);
void addParameter(std::string const &key, T&& value) {
mParams.Add<T>(key, std::forward<T>(value));
}
......
/********************************************************************************
* Copyright (c) 2023 CEA-List
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* SPDX-License-Identifier: EPL-2.0
*
********************************************************************************/
#ifndef AIDGE_ANY_H_
#define AIDGE_ANY_H_
#include <typeinfo> // typeid
#include <type_traits> // enable_if_t, decay_t, is_same, is_copy_constructible, remove_cv, remove_reference
#include <assert.h>
#include <new>
class _any {
private:
enum _Op { _Op_access, _Op_get_type_info, _Op_clone, _Op_destroy };
union _Arg {
const std::type_info* _M_typeinfo;
_any* _M_any;
};
void* _M_data;
void (*_M_manager)(_Op, const _any*, _Arg*);
public:
template <typename Tp>
struct Manager {
static void manage(_Op which, const _any* __any, _Arg* __arg) {
auto ptr = static_cast<const Tp*>(__any->_M_data);
switch (which)
{
case _Op_get_type_info:
__arg->_M_typeinfo = &typeid(Tp);
break;
case _Op_clone:
__arg->_M_any->_M_data = new Tp(*ptr);
__arg->_M_any->_M_manager = __any->_M_manager;
break;
case _Op_destroy:
delete ptr;
break;
}
}
static Tp* access(const _any* __any) {
return static_cast<Tp*>(__any->_M_data);
}
// template <typename Up>
// static void create(void* data, Up&& value) {
// data = new Tp(std::forward<Up>(value));
// }
};
private:
template<typename _Tp, typename _VTp = std::decay_t<_Tp>>
using _Decay_if_not_any = std::enable_if_t<!std::is_same<_VTp, _any>::value, _VTp>;
public:
_any() noexcept : _M_manager(nullptr) { }
_any(const _any& __other)
{
if (!__other._M_manager)
_M_manager = nullptr;
else
{
_Arg __arg;
__arg._M_any = this;
__other._M_manager(_Op_clone, &__other, &__arg);
}
}
_any(_any&& __other)
{
if (!__other._M_manager)
_M_manager = nullptr;
else
{
_M_data = __other._M_data;
_M_manager = __other._M_manager;
const_cast<_any*>(&__other)->_M_manager = nullptr;
}
}
template<typename T, typename VT = _Decay_if_not_any<T>, std::enable_if_t<std::is_copy_constructible<VT>::value, bool> = true>
explicit _any(T&& value)
: _M_manager(&Manager<VT>::manage),
_M_data(new VT{std::forward<T>(value)})
{}
~_any()
{
if(_M_manager) {
_M_manager(_Op_destroy, this, nullptr);
_M_manager = nullptr;
}
}
const std::type_info& type() const
{
if (!_M_manager)
return typeid(void);
_Arg __arg;
_M_manager(_Op_get_type_info, this, &__arg);
return *__arg._M_typeinfo;
}
};
template<typename _ValueType>
inline _ValueType any_cast(const _any& __any)
{
using _Up = std::remove_cv_t<std::remove_reference_t<_ValueType>>;
assert((std::__or_<std::is_reference<_ValueType>, std::is_copy_constructible<_ValueType>>::value && "Template argument must be a reference or CopyConstructible type"));
assert((std::is_constructible<_ValueType, const _Up&>::value && "Template argument must be constructible from a const value."));
assert(std::is_object<_Up>::value);
assert(__any.type() == typeid(_Up));
auto __p = static_cast<_Up*>(__any._M_data);
if (__p)
return static_cast<_ValueType>(*__p);
throw std::bad_cast();
}
#endif /* AIDGE_ANY_H_ */
\ No newline at end of file
......@@ -12,23 +12,30 @@
#ifndef AIDGE_CPARAMETER_H_
#define AIDGE_CPARAMETER_H_
#include <assert.h>
#include <map>
#include <vector>
#include "aidge/utils/Any.hpp"
namespace Aidge {
///\todo store also a fix-sized code that indicates the type
///\todo managing complex types or excluding non-trivial, non-aggregate types
class CParameter
{
class CParameter {
private:
template <typename T>
struct is_vector : std::false_type {};
template <typename T, typename Alloc>
struct is_vector<std::vector<T, Alloc>> : std::true_type {};
template<typename _ValueType>
inline _ValueType& any_cast_ref(const _any& __any)
{
using _Up = std::remove_cv_t<std::remove_reference_t<_ValueType>>;
assert((std::__or_<std::is_reference<_ValueType>, std::is_copy_constructible<_ValueType>>::value && "Template argument must be a reference or CopyConstructible type"));
assert((std::is_constructible<_ValueType, const _Up&>::value && "Template argument must be constructible from a const value."));
assert(std::is_object<_Up>::value);
assert(__any.type() == typeid(_Up));
if (_any::Manager<_Up>::access(&__any)) {
return *static_cast<_ValueType*>(_any::Manager<_Up>::access(&__any));
}
throw std::bad_cast();
}
public:
// not copyable, not movable
CParameter(CParameter const &) = delete;
......@@ -48,15 +55,16 @@ public:
* param buffer that will get invalid after the CParam death.
* \note at() throws if the parameter does not exist, using find to test for parameter existance
*/
template<class T> T Get(std::string const i_ParamName) const
template<class T> T& Get(const std::string i_ParamName)
{
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(typeid(T).name() == m_Types.at(i_ParamName));
return *reinterpret_cast<T *>(m_BeginBuffer + m_Params.at(i_ParamName));
return any_cast_ref<T>(m_Buffer[m_Params.at(i_ParamName)]);
}
// template<class T> const T& Get(const std::string i_ParamName) const
// {
// return any_cast<T>(m_Buffer[m_Params.at(i_ParamName)]);
// }
///\brief Add a parameter value, identified by its name
///\tparam T expected parameter type
///\param i_ParamName Parameter name
......@@ -64,16 +72,10 @@ public:
///\todo Pass i_Value by ref if large or not trivial
///\bug If parameter already exists, its value is changed but written in the
/// 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)
template<class T> void Add(const std::string &i_ParamName, T&& 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
m_Types[i_ParamName] = typeid(i_Value).name();
m_Params[i_ParamName] = m_Buffer.size(); // Copy pointer offset
m_Buffer.push_back(_any(std::forward<T>(i_Value)));
}
......@@ -100,14 +102,7 @@ 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::vector<_any> m_Buffer = {};
};
}
......
......@@ -20,10 +20,10 @@ using namespace Aidge;
TEST_CASE("[core/operators] GenericOp(add & get parameters)", "[Operator]") {
SECTION("INT") {
GenericOperator_Op Testop("TestOp", 1, 1, 1);
int value = 5;
const char* key = "intParam";
Testop.addParameter(key, value);
REQUIRE(Testop.getParameter<int>(key) == value);
Testop.addParameter(key, int(5));
int registeredVal = Testop.getParameter<int>(key);
REQUIRE(registeredVal == 5);
}
SECTION("LONG") {
GenericOperator_Op Testop("TestOp", 1, 1, 1);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment