Bugfix/479

python_tests/test_issues_12.py

@@ -34,7 +34,7 @@ def get_random_tax_id(tax_ids_set=set()):
 
 
 @pytest.mark.stress_test
-@pytest.mark.parametrize("db0_slab_size", [{"slab_size": 64 << 20, "autocommit": False}], indirect=True)
+@pytest.mark.parametrize("db0_slab_size", [{"slab_size": 256 << 20, "autocommit": False}], indirect=True)
 def test_no_cache_allocator_issue(db0_slab_size):
     db0.set_cache_size(8 << 30)
     # create 25 k unique tax_id numbers
@@ -55,7 +55,7 @@ def test_no_cache_allocator_issue(db0_slab_size):
         new_issuer = Issuer(tax_id=tax_id, inv_list=[], inv_index=db0.index())
         issuers[tax_id] = new_issuer
 
-    execution_time = 15
+    execution_time = 45
     RANDOM_BYTES = b'DB0'*22000
     total_size = 0
     count_of_objects = 0

run_memcheck.sh

@@ -3,5 +3,6 @@ export PYTHONIOENCODING=utf8
 export G_SLICE=always-malloc 
 export G_DEBUG=gc-friendly
 #valgrind -v --tool=memcheck --leak-check=no --num-callers=250 --log-file=valgrind.log python3 -m pytest -m 'not integration_test' -m 'not stress_test' -c pytest.ini --capture=no "$@"
-valgrind -v --tool=memcheck --leak-check=no --num-callers=250 --log-file=valgrind.log python3 -m pytest -m 'stress_test' -c pytest.ini --capture=no "$@"
-# valgrind -v --tool=memcheck --leak-check=full --num-callers=40 --log-file=valgrind.log python3 -m samples.explore --path='/src/zorch/app-data'
+#valgrind -v --tool=memcheck --leak-check=no --num-callers=250 --log-file=valgrind.log python3 -m pytest -m 'stress_test' -c pytest.ini --capture=no "$@"
+#valgrind -v --tool=memcheck --leak-check=full --num-callers=40 --log-file=valgrind.log python3 -m samples.explore --path='/src/zorch/app-data'
+valgrind -v --tool=memcheck --leak-check=no --num-callers=250 --log-file=valgrind.log python3 -m pytest -m 'stress_test' -k='test_no_cache_allocator_issue' -c pytest.ini --capture=no "$@"

src/dbzero/bindings/python/collections/PyIndex.cpp

@@ -70,6 +70,18 @@ namespace db0::python
         auto py_index = Py_OWN(IndexDefaultObject_new());
         db0::FixtureLock lock(PyToolkit::getPyWorkspace().getWorkspace().getCurrentFixture());
         auto &index = py_index->makeNew(*lock);
+
+        // NOTE: this callback is important for proper lifecycle management
+        // we must prevent dirty Index instance from deletion
+        auto py_index_ptr = py_index.get();
+        index.setDirtyCallback([py_index_ptr](bool incRef) {
+            if (incRef) {
+                Py_INCREF(py_index_ptr);
+            } else {
+                Py_DECREF(py_index_ptr);
+            }
+        });
+
         // register newly created index with py-object cache
         lock->getLangCache().add(index.getAddress(), py_index.get());
         return py_index.steal();

src/dbzero/object_model/index/Index.cpp

@@ -265,7 +265,17 @@ namespace db0::object_model
                     << static_cast<std::uint16_t>((*this)->m_data_type) << THROWF_END;
         }
     }
-
+
+    void Index::setDirty(bool dirty)
+    {
+        if (dirty) {
+            getMemspace().collectForFlush(this);            
+        }
+        if (m_dirty_callback) {
+            m_dirty_callback(dirty);
+        }
+    }
+
     void Index::add(ObjectPtr key, ObjectPtr value)
     {        
         assert(hasInstance());
@@ -283,7 +293,7 @@ namespace db0::object_model
 
         // subscribe for flush operation
         if (!isDirty()) {
-            getMemspace().collectForFlush(this);
+            setDirty(true);
         }
 
         switch (m_builder.getDataType()) {
@@ -323,7 +333,7 @@ namespace db0::object_model
 
         // subscribe for flush operation
         if (!isDirty()) {
-            getMemspace().collectForFlush(this);
+            setDirty(true);            
         }
 
         switch (m_builder.getDataType()) {
@@ -443,7 +453,7 @@ namespace db0::object_model
         assert(hasInstance());
         // subscribe for flush operation
         if (!isDirty()) {
-            getMemspace().collectForFlush(this);
+            setDirty(true);
         }
 
         switch (m_builder.getDataType()) {
@@ -489,14 +499,15 @@ namespace db0::object_model
         }
         return type_manager.extractUInt64(type_manager.getTypeId(value), value);
     }
-
+    
     void Index::flush(bool revert)
     {
         if (revert) {
             rollback();
         } else {
             _flush();
-        }        
+        }
+        setDirty(false);
     }
 
     void Index::flushOp(void *ptr, bool revert) {
@@ -506,7 +517,7 @@ namespace db0::object_model
     void Index::removeNull(ObjectPtr obj_ptr)
     {
         if (!isDirty()) {
-            getMemspace().collectForFlush(this);
+            setDirty(true);
         }
 
         switch (m_builder.getDataType()) {

src/dbzero/object_model/index/Index.hpp

@@ -79,13 +79,24 @@ namespace db0::object_model
         void rollback();
 
         void operator=(const Index &) = delete;
-
+
+        // Callback invoked when the index is marked dirty (or clean)
+        // for proper lifecycle management (incRef prevents premature deletion e.g. from LangCache)
+        // @param incRef true to increase ref count, false to decrease        
+        void setDirtyCallback(std::function<void(bool incRef)> &&callback) {
+            m_dirty_callback = std::move(callback);
+        }
+
     protected:
         // the default / provisional type
         using DefaultT = std::int64_t;
         friend struct Builder;
         void flush(bool revert);
         static void flushOp(void *, bool revert);
+        std::function<void(bool incRef)> m_dirty_callback;
+
+        // Set or reset dirty state
+        void setDirty(bool dirty);
 
         template <typename T> static constexpr IndexDataType dataTypeOf()
         {

tests/utils/cp_data_1.cpp

@@ -1,4 +1,5 @@
 #include "cp_data_1.hpp"
+#include <tuple>
 
 namespace db0::tests
 
@@ -9,7 +10,7 @@ namespace db0::tests
     {
         return {
             {0,0,0,2},
-            {0,0,0,4},
+            {0,0,0,4},            
             {0,1,66965504,1},
             {0,0,66965504,6},
             {0,0,66965504,8},
@@ -875,7 +876,7 @@ namespace db0::tests
             {1,0,66965504,470},
             {2,0,54580772,470},
             {1,0,66965504,472},
-            {2,0,54580772,472},
+            {2,0,54580772,472},            
             {1,0,66965504,474},
             {2,0,54580772,474},
             {1,0,66965504,476},
@@ -1317,7 +1318,7 @@ namespace db0::tests
             {1,0,54580772,560},
             {2,0,54583076,560},
             // failing operation
-            {1,0,54580772,788},
+            {1,0,54580772,788},            
         };
     }