Memory leak in Equilibrium

[JasonChmn]

We instantiate a new Solver_LP_abstract and never delete it.

hpp-centroidal-dynamics/src/centroidal_dynamics.cpp

Line 57 in 830c6ef

m_solver = Solver_LP_abstract::getNewSolver(solver_type);

Result on valgrind :

==21177== at 0x4C2E0EF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21177== by 0xD6EEC51: centroidal_dynamics::Solver_LP_abstract::getNewSolver(centroidal_dynamics::SolverLP) (in /opt/openrobots/lib/libhpp-centroidal-dynamics.so)
==21177== by 0xD6E030B: centroidal_dynamics::Equilibrium::Equilibrium(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, double, unsigned int, centroidal_dynamics::SolverLP, bool, unsigned int, bool) (in /opt/openrobots/lib/libhpp-centroidal-dynamics.so)

Adding a destructor in Equilibrium class deleting the pointer m_solver to Solver_LP_abstract , create a new memory leak with qpOASES when calling function solve() of solver_LP :

hpp-centroidal-dynamics/src/solver_LP_qpoases.cpp

Line 22 in 830c6ef

LP_status Solver_LP_qpoases::solve(Cref_vectorX c, Cref_vectorX lb, Cref_vectorX ub, Cref_matrixXX A, Cref_vectorX Alb,

==18727==    at 0x4C2E80F: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18727==    by 0x8B3AC70: qpOASES::QProblem::copy(qpOASES::QProblem const&) (in /opt/openrobots/lib/libqpOASES.so.3.2)
==18727==    by 0x8B3AF9B: qpOASES::QProblem::operator=(qpOASES::QProblem const&) (in /opt/openrobots/lib/libqpOASES.so.3.2)
==18727==    by 0x8B1404D: qpOASES::SQProblem::operator=(qpOASES::SQProblem const&) (in /opt/openrobots/lib/libqpOASES.so.3.2)
==18727==    by 0xD8F553D: centroidal_dynamics::Solver_LP_qpoases::solve(Eigen::Ref<Eigen::Matrix<double, -1, 1, 0, -1, 1> const, 0, Eigen::InnerStride<1> > const&, Eigen::Ref<Eigen::Matrix<double, -1, 1, 0, -1, 1> const, 0, Eigen::InnerStride<1> > const&, Eigen::Ref<Eigen::Matrix<double, -1, 1, 0, -1, 1> const, 0, Eigen::InnerStride<1> > const&, Eigen::Ref<Eigen::Matrix<double, -1, -1, 1, -1, -1> const, 0, Eigen::OuterStride<-1> > const&, Eigen::Ref<Eigen::Matrix<double, -1, 1, 0, -1, 1> const, 0, Eigen::InnerStride<1> > const&, Eigen::Ref<Eigen::Matrix<double, -1, 1, 0, -1, 1> const, 0, Eigen::InnerStride<1> > const&, Eigen::Ref<Eigen::Matrix<double, -1, 1, 0, -1, 1>, 0, Eigen::InnerStride<1> >) (in /opt/openrobots/lib/libhpp-centroidal-dynamics.so)
==18727==    by 0xD8A4C28: centroidal_dynamics::Equilibrium::computeEquilibriumRobustness(Eigen::Ref<Eigen::Matrix<double, 3, 1, 0, 3, 1> const, 0, Eigen::InnerStride<1> > const&, Eigen::Ref<Eigen::Matrix<double, 3, 1, 0, 3, 1> const, 0, Eigen::InnerStride<1> > const&, double&) (in /opt/openrobots/lib/libhpp-centroidal-dynamics.so)

[JasonChmn]

The problem was when changing the member :

hpp-centroidal-dynamics/include/hpp/centroidal-dynamics/solver_LP_qpoases.hh

Line 19 in 830c6ef

qpOASES::SQProblem m_solver; // qpoases solver

hpp-centroidal-dynamics/src/solver_LP_qpoases.cpp

Line 35 in 830c6ef

m_solver = SQProblem(n, m, HST_ZERO);

The '=' operator of qpOASES::SQProblem seems to allocate memory in their copy function which is lost then (memory leak).
A solution to avoid using the '=' operator is to replace the member qpOASES::SQProblem m_solver; by a pointer qpOASES::SQProblem * m_solver;.
Then, instead of doing m_solver = SQProblem(n, m, HST_ZERO);, do :

# 1- Delete the previous pointer m_solver if not NULL
# 2- Create a new SQProblem 
m_solver = new SQProblem(n, m, HST_ZERO);
# 3- Delete this m_solver if not NULL when solver_LP_qpoases is deleted

Remarks :
Do we really need to create a new SQProblem at each call of solve function ?

hpp-centroidal-dynamics/src/solver_LP_qpoases.cpp

Line 35 in 830c6ef

m_solver = SQProblem(n, m, HST_ZERO);

Or can we instead do it just once in the constructor ?
Doing so, would imply that we know the value of n and m in the constructor :

hpp-centroidal-dynamics/src/solver_LP_qpoases.cpp

Line 24 in 830c6ef

int n = (int)c.size(); // number of variables

[jmirabel]

As far as I understand this code, your proposal should work. I believe that qpoases would be happy to have a minimal not-working example of this. If you do so, you may post it here: coin-or/qpOASES#30

Do we really need to create a new SQProblem at each call of solve function ?

The SQProblem is rebuilt only is the dimension of the problem changed.

[JasonChmn]

After further investigation, the problem does not come from qpOASES but from our implementation.
Adding some destructors fix it : #11