#1050 Memory leak in StringPool code (#1058)

* stringpool fix * stringpool fix * stringpool fix * stringpool fix * stringpool fix * stringpool fix * stringpool fix * smart pointer fix * clang fixes * disable cpp unittests --------- Co-authored-by: Roman Porozhnetov <roman_porozhnetov@epam.com>
epam · Mar 13, 2023 · 96d084a · 96d084a
1 parent d904adb
commit 96d084a
Show file tree

Hide file tree

Showing 9 changed files with 22 additions and 42 deletions.
diff --git a/api/c/indigo-renderer/src/indigo_render2d.cpp b/api/c/indigo-renderer/src/indigo_render2d.cpp
@@ -33,7 +33,7 @@
 #include "indigo_renderer_internal.h"
 #include "option_manager.h"
 
-//#define INDIGO_DEBUG
+// #define INDIGO_DEBUG
 
 #ifdef INDIGO_DEBUG
 #include <iostream>
@@ -488,18 +488,18 @@ CEXPORT int indigoRender(int object, int output)
         if (IndigoBaseMolecule::is(obj))
         {
             if (obj.getBaseMolecule().isQueryMolecule())
-                rp.mol = std::make_unique<QueryMolecule>();
+                rp.mol.reset(new QueryMolecule());
             else
-                rp.mol = std::make_unique<Molecule>();
+                rp.mol.reset(new Molecule());
             rp.mol->clone_KeepIndices(self.getObject(object).getBaseMolecule());
             rp.rmode = RENDER_MOL;
         }
         else if (IndigoBaseReaction::is(obj))
         {
             if (obj.getBaseReaction().isQueryReaction())
-                rp.rxn = std::make_unique<QueryReaction>();
+                rp.rxn.reset(new QueryReaction());
             else
-                rp.rxn = std::make_unique<Reaction>();
+                rp.rxn.reset(new Reaction());
             rp.rxn->clone(self.getObject(object).getBaseReaction(), 0, 0, 0);
             rp.rmode = RENDER_RXN;
         }

diff --git a/api/c/tests/unit/CMakeLists.txt b/api/c/tests/unit/CMakeLists.txt
@@ -4,11 +4,10 @@ project(indigo-api-unit-tests LANGUAGES CXX)
 
 if (ENABLE_TESTS)
     if(UNIX AND NOT APPLE)
-    #    commented out due to memory leaks
-    #    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -fsanitize=leak -g")
-    #    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -fsanitize=leak -g")
-    #    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address -fsanitize=leak")
-    #    set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -fsanitize=address -fsanitize=leak")
+        # set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=leak -g")
+        # set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=leak -g")
+        # set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=leak")
+        # set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -fsanitize=leak")
     endif()
     set(DATA_PATH ${CMAKE_SOURCE_DIR}/data)
     configure_file(${CMAKE_CURRENT_SOURCE_DIR}/common.h.in ${CMAKE_CURRENT_BINARY_DIR}/common.h)

diff --git a/api/cpp/tests/CMakeLists.txt b/api/cpp/tests/CMakeLists.txt
@@ -4,11 +4,11 @@ project(indigo-cpp-unit-tests LANGUAGES CXX)
 
 if (ENABLE_TESTS)
     if(UNIX AND NOT APPLE)
-    #    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -fsanitize=leak -g")
-    #    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -fsanitize=leak -g")
-    #    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address -fsanitize=leak")
-    #    set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -fsanitize=address -fsanitize=leak")
-    #    set(CMAKE_CXX_COMPILER_LAUNCHER ${CMAKE_COMMAND} -E env ASAN_OPTIONS=strict_string_checks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1)
+        # set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -fsanitize=leak -g")
+        # set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address -fsanitize=leak -g")
+        # set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address -fsanitize=leak")
+        # set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -fsanitize=address -fsanitize=leak")
+        # set(CMAKE_CXX_COMPILER_LAUNCHER ${CMAKE_COMMAND} -E env ASAN_OPTIONS=strict_string_checks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1)
     endif()
     set(DATA_PATH ${CMAKE_SOURCE_DIR}/data)
     file(GLOB_RECURSE ${PROJECT_NAME}_SOURCES CONFIGURE_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/**/*.cpp)

diff --git a/core/indigo-core/common/base_cpp/string_pool.cpp b/core/indigo-core/common/base_cpp/string_pool.cpp
@@ -41,14 +41,12 @@ int StringPool::_add(const char* str, int size)
 
     if (idx >= _storage.size())
         _storage.resize(idx + 1);
-    if (_storage.at(idx) == 0)
-        _storage.set(idx, new Array<char>());
     if (size == -1 && str == 0)
         throw Error("Internal error: size == -1 && str == 0");
 
     if (size == -1)
         size = strlen(str);
-    _storage.at(idx)->resize(size + 1);
+    _storage.at(idx).resize(size + 1);
     if (str != 0 && size != 0)
         memcpy(at(idx), str, size);
     at(idx)[size] = 0;
@@ -77,12 +75,12 @@ void StringPool::remove(int idx)
 
 char* StringPool::at(int idx)
 {
-    return _storage[_pool[idx]]->ptr();
+    return _storage[_pool[idx]].ptr();
 }
 
 const char* StringPool::at(int idx) const
 {
-    return _storage[_pool[idx]]->ptr();
+    return _storage[_pool[idx]].ptr();
 }
 
 int StringPool::size() const

diff --git a/core/indigo-core/common/base_cpp/string_pool.h b/core/indigo-core/common/base_cpp/string_pool.h
@@ -20,8 +20,8 @@
 #define __string_pool_h__
 
 #include "base_cpp/auto_iter.h"
+#include "base_cpp/obj_array.h"
 #include "base_cpp/pool.h"
-#include "base_cpp/ptr_array.h"
 
 #ifdef _WIN32
 #pragma warning(push)
@@ -96,7 +96,7 @@ namespace indigo
         int _add(const char* str, int size);
 
         Pool<int> _pool;
-        PtrArray<Array<char>> _storage;
+        ObjArray<Array<char>> _storage;
 
     private:
         StringPool(const StringPool&); // no implicit copy

diff --git a/core/indigo-core/molecule/molecule.h b/core/indigo-core/molecule/molecule.h
@@ -33,8 +33,6 @@ namespace indigo
     {
     public:
         Molecule();
-        ~Molecule() override;
-
         Molecule& asMolecule() override;
 
         void clear() override;

diff --git a/core/indigo-core/molecule/src/molecule.cpp b/core/indigo-core/molecule/src/molecule.cpp
@@ -33,10 +33,6 @@ Molecule::Molecule()
     _ignore_bad_valence = false;
 }
 
-Molecule::~Molecule()
-{
-}
-
 Molecule& Molecule::asMolecule()
 {
     return *this;

diff --git a/core/render2d/src/render_cdxml.cpp b/core/render2d/src/render_cdxml.cpp
@@ -149,20 +149,11 @@ void RenderParamCdxmlInterface::render(RenderParams& params)
 void RenderParamCdxmlInterface::_renderRxns(RenderParams& params)
 {
     ReactionCdxmlSaver saver(*params.rOpt.output);
-
-    Array<BaseReaction*> rxns;
-
     if (params.rxns.size() != 0)
         for (int i = 0; i < params.rxns.size(); ++i)
-            rxns.push(params.rxns[i]);
-    else if (params.rxn.get() != 0)
-        rxns.push(params.rxn.get());
-
-    for (int rxn_ind = 0; rxn_ind < rxns.size(); rxn_ind++)
-    {
-        BaseReaction& rxn = (BaseReaction&)*rxns[rxn_ind];
-        saver.saveReaction(rxn);
-    }
+            saver.saveReaction(*params.rxns[i]);
+    else if (params.rxn)
+        saver.saveReaction(*params.rxn);
 }
 
 void RenderParamCdxmlInterface::_renderMols(RenderParams& params)

diff --git a/core/render2d/src/render_params.cpp b/core/render2d/src/render_params.cpp
@@ -69,8 +69,6 @@ void RenderParams::clear()
     relativeThickness = 1.0f;
     bondLineWidthFactor = 1.0f;
     rmode = RENDER_NONE;
-    mol.reset(nullptr);
-    rxn.reset(nullptr);
     rOpt.clear();
     cnvOpt.clear();
     clearArrays();