Fixed all memory leaks. Probably I don't understand how EXPRTK really

works. using smart pointers with EXPRTK caused memory leak.

Author: Dilawar Singh

builtins/MooseParser.cpp

@@ -23,9 +23,7 @@ using namespace std;
 namespace moose
 {
 
-MooseParser::MooseParser() : 
-    expression_(new  moose::Parser::expression_t())
-    , symbol_tables_registered_(false)
+MooseParser::MooseParser()
 {
     Parser::symbol_table_t symbol_table;
 
@@ -39,13 +37,14 @@ MooseParser::MooseParser() :
     symbol_table.add_function( "srand2", MooseParser::SRand2 );
     symbol_table.add_function( "fmod", MooseParser::Fmod );
 
-    expression_->register_symbol_table(symbol_table);
+    expression_.register_symbol_table(symbol_table);
 
 }
 
 MooseParser::~MooseParser()
 {
-    expression_->release();
+    ClearAll();
+    expression_.release();
 }
 
 /*-----------------------------------------------------------------------------
@@ -89,11 +88,17 @@ double MooseParser::Fmod( double a, double b )
 /*-----------------------------------------------------------------------------
  *  Get/Set
  *-----------------------------------------------------------------------------*/
-Parser::symbol_table_t& MooseParser::GetSymbolTable( ) const
+Parser::symbol_table_t& MooseParser::GetSymbolTable()
 {
-    return expression_->get_symbol_table();
+    return expression_.get_symbol_table();
 }
 
+const Parser::symbol_table_t& MooseParser::GetSymbolTable() const
+{
+    return expression_.get_symbol_table();
+}
+
+
 double MooseParser::GetVarValue(const string& name) const
 {
     return GetSymbolTable().get_variable(name)->value();
@@ -211,7 +216,7 @@ bool MooseParser::CompileExpr()
     ASSERT_FALSE(expr_.empty(), __func__ << ": Empty expression not allowed here");
 
     Parser::parser_t  parser;
-    auto res = parser.compile(expr_, *expression_);
+    auto res = parser.compile(expr_, expression_);
     if(! res)
     {
         std::stringstream ss;
@@ -240,14 +245,14 @@ bool MooseParser::CompileExpr()
 
 double MooseParser::Derivative(const string& name) const
 {
-    return exprtk::derivative(*expression_, name);
+    return exprtk::derivative(expression_, name);
 }
 
 double MooseParser::Eval(bool check) const
 {
     if( expr_.empty())
         return 0.0;
-    double v = expression_->value();
+    double v = expression_.value();
     if(check)
     {
         if(! std::isfinite(v)) 
@@ -289,7 +294,7 @@ void MooseParser::ClearAll( )
 
 void MooseParser::Reset( )
 {
-    expression_->release();
+    expression_.release();
 }
 
 const string MooseParser::GetExpr( ) const

builtins/MooseParser.h

@@ -60,7 +60,8 @@ class MooseParser
     /*-----------------------------------------------------------------------------
      *  Set/Get
      *-----------------------------------------------------------------------------*/
-    Parser::symbol_table_t& GetSymbolTable( ) const;
+    Parser::symbol_table_t& GetSymbolTable();
+    const Parser::symbol_table_t& GetSymbolTable() const;
 
     void SetSymbolTable( Parser::symbol_table_t tab );
 
@@ -122,9 +123,8 @@ class MooseParser
     /* Map to variable names and pointer to their values. */
     // map<string, double*> refs_;
 
-    Parser::expression_t* expression_;     /* expression type */
+    Parser::expression_t expression_;     /* expression type */
     size_t num_user_defined_funcs_ = 0;
-
     bool symbol_tables_registered_;
 };
 

ksolve/FuncTerm.cpp

@@ -33,18 +33,20 @@ FuncTerm::FuncTerm():
 
 FuncTerm::~FuncTerm()
 {
+    if(args_)
+        delete[] args_;
 }
 
 void FuncTerm::setReactantIndex( const vector< unsigned int >& mol )
 {
     reactantIndex_ = mol;
     if ( args_ )
     {
-        parser_.Reset();
-        // parser_.ClearAll();
+        delete[] args_;
+        parser_.ClearAll();
     }
 
-    args_.reset(new double[mol.size()+1]);
+    args_ = new double[mol.size()+1];
     for ( unsigned int i = 0; i < mol.size(); ++i )
     {
         args_[i] = 0.0;
@@ -123,7 +125,7 @@ double FuncTerm::getVolScale() const
 
 const FuncTerm& FuncTerm::operator=( const FuncTerm& other )
 {
-    args_ = nullptr;
+    args_ = nullptr;  // other is still using it.
     expr_ = other.expr_;
     volScale_ = other.volScale_;
     target_ = other.target_;
@@ -139,7 +141,7 @@ const FuncTerm& FuncTerm::operator=( const FuncTerm& other )
  */
 double FuncTerm::operator() ( const double* S, double t ) const
 {
-    if ( ! args_.get() )
+    if ( ! args_ )
         return 0.0;
 
     unsigned int i = 0;
@@ -160,7 +162,7 @@ double FuncTerm::operator() ( const double* S, double t ) const
 
 void FuncTerm::evalPool( double* S, double t ) const
 {
-    if ( !args_.get() || target_ == ~0U )
+    if ( !args_ || target_ == ~0U )
         return;
 
     unsigned int i;

ksolve/FuncTerm.h

@@ -53,7 +53,7 @@ class FuncTerm
     double volScale_;
     unsigned int target_; /// Index of the entity to be updated by Func
 
-    unique_ptr<double[]> args_;
+    double* args_;
 
     string expr_;
     moose::MooseParser parser_;

tests/py_rdesigneur/test_72_CICR.py

@@ -85,8 +85,8 @@ def test():
     # print(np.array_repr(s))
     # In multithreaded mode, the numers are not exactly the same as in
     # expected.
-    assert np.allclose(m, E[0], rtol=1e-2), (m - E[0])
-    assert np.allclose(s, E[1], rtol=1e-2), (s - E[1])
+    assert np.allclose(m, E[0], rtol=1e-2, atol=1e-4), (m - E[0])
+    assert np.allclose(s, E[1], rtol=1e-2, atol=1e-4), (s - E[1])
     print('done')