From 1c0c5e3ab29258f392c1e0a6c305c0987bb6d94a Mon Sep 17 00:00:00 2001 From: Wojtek Date: Mon, 15 Dec 2025 20:59:52 +0100 Subject: [PATCH] fix:list detach issue --- python_tests/test_refresh.py | 8 +++-- run_asan_stress_tests.sh | 1 + run_asan_tests.sh | 1 + .../bindings/python/iter/PyObjectIterable.cpp | 3 +- .../bindings/python/iter/PyObjectIterator.cpp | 5 +-- src/dbzero/object_model/dict/Dict.cpp | 4 +++ src/dbzero/object_model/dict/Dict.hpp | 3 +- src/dbzero/object_model/dict/DictIterator.cpp | 7 +++- src/dbzero/object_model/dict/DictIterator.hpp | 2 +- .../object_model/iterators/BaseIterator.hpp | 32 +++++++++++++++++-- src/dbzero/object_model/list/List.cpp | 9 ++++++ src/dbzero/object_model/list/List.hpp | 2 ++ src/dbzero/object_model/list/ListIterator.cpp | 3 +- src/dbzero/object_model/list/ListIterator.hpp | 4 +-- src/dbzero/object_model/set/Set.cpp | 6 +++- src/dbzero/object_model/set/Set.hpp | 3 +- src/dbzero/object_model/set/SetIterator.cpp | 4 ++- src/dbzero/object_model/set/SetIterator.hpp | 3 +- .../object_model/tuple/TupleIterator.cpp | 3 +- .../object_model/tuple/TupleIterator.hpp | 6 +++- 20 files changed, 87 insertions(+), 22 deletions(-) diff --git a/python_tests/test_refresh.py b/python_tests/test_refresh.py index 6978c338..e8156510 100644 --- a/python_tests/test_refresh.py +++ b/python_tests/test_refresh.py @@ -606,6 +606,7 @@ def validate_current_prefix(expected_len = None, expected_min_len = None): db0.init(DB0_DIR) db0.open(px_name, "r") last_len = 0 + time.sleep(2.0) while True: try: root = db0.fetch(MemoTestSingleton) @@ -636,11 +637,11 @@ def validate_current_prefix(expected_len = None, expected_min_len = None): @pytest.mark.stress_test -@pytest.mark.skip(reason="Test disabled due to issue #605") +# @pytest.mark.skip(reason="Test disabled due to issue #605") # test failing due to issue: https://github.com/dbzero-software/dbzero/issues/605 def test_refresh_prefix_continuous_process(db0_fixture): px_name = db0.get_current_prefix().name - + def validate_current_prefix(expected_len = None, expected_min_len = None): root = db0.fetch(MemoTestSingleton) assert not expected_min_len or len(root.value) >= expected_min_len @@ -668,6 +669,9 @@ def validate_current_prefix(expected_len = None, expected_min_len = None): last_len = 0 while True: try: + if not db0.exists(MemoTestSingleton): + time.sleep(0.1) + continue root = db0.fetch(MemoTestSingleton) if len(root.value) > 1: last_len = len(root.value) diff --git a/run_asan_stress_tests.sh b/run_asan_stress_tests.sh index 73e0907f..90978a28 100755 --- a/run_asan_stress_tests.sh +++ b/run_asan_stress_tests.sh @@ -1,5 +1,6 @@ #!/bin/bash export PYTHONIOENCODING=utf8 export LD_PRELOAD=$(gcc -print-file-name=libasan.so) +export PYTHONMALLOC=malloc python3 -m pytest -m 'stress_test' -c pytest.ini --capture=no "$@" diff --git a/run_asan_tests.sh b/run_asan_tests.sh index cd9c70e2..b44be10f 100755 --- a/run_asan_tests.sh +++ b/run_asan_tests.sh @@ -1,5 +1,6 @@ #!/bin/bash export PYTHONIOENCODING=utf8 export LD_PRELOAD=$(gcc -print-file-name=libasan.so) +export PYTHONMALLOC=malloc python3 -m pytest -m 'not integration_test' -m 'not stress_test' -c pytest.ini --capture=no "$@" -vv diff --git a/src/dbzero/bindings/python/iter/PyObjectIterable.cpp b/src/dbzero/bindings/python/iter/PyObjectIterable.cpp index 8aca98ae..e2b1647d 100644 --- a/src/dbzero/bindings/python/iter/PyObjectIterable.cpp +++ b/src/dbzero/bindings/python/iter/PyObjectIterable.cpp @@ -20,6 +20,7 @@ namespace db0::python void PyObjectIterable_del(PyObjectIterable* self) { + PY_API_FUNC // destroy associated db0 instance self->destroy(); Py_TYPE(self)->tp_free((PyObject*)self); @@ -28,7 +29,7 @@ namespace db0::python shared_py_object PyObjectIterableDefault_new() { return { PyObjectIterable_new(&PyObjectIterableType, NULL, NULL), false }; } - + PyObject *tryPyObjectIterable_compare(PyObject *self, PyObject* const *args, Py_ssize_t nargs) { if (nargs != 1) { diff --git a/src/dbzero/bindings/python/iter/PyObjectIterator.cpp b/src/dbzero/bindings/python/iter/PyObjectIterator.cpp index 17900f27..6bbe2aae 100644 --- a/src/dbzero/bindings/python/iter/PyObjectIterator.cpp +++ b/src/dbzero/bindings/python/iter/PyObjectIterator.cpp @@ -14,14 +14,15 @@ namespace db0::python { using ObjectIterator = db0::object_model::ObjectIterator; - + PyObjectIterator *PyObjectIterator_new(PyTypeObject *type, PyObject *, PyObject *) { return reinterpret_cast(type->tp_alloc(type, 0)); } void PyObjectIterator_del(PyObjectIterator* self) { - // destroy associated db0 instance + PY_API_FUNC + // destroy associated instance self->destroy(); Py_TYPE(self)->tp_free((PyObject*)self); } diff --git a/src/dbzero/object_model/dict/Dict.cpp b/src/dbzero/object_model/dict/Dict.cpp index d6b118f5..c12309f0 100644 --- a/src/dbzero/object_model/dict/Dict.cpp +++ b/src/dbzero/object_model/dict/Dict.cpp @@ -261,6 +261,10 @@ namespace db0::object_model // FIXME: can be removed when GC0 calls commit-op commit(); m_index.detach(); + // detach all associated iterators + m_iterators.forEach([](DictIterator &iter) { + iter.detach(); + }); super_t::detach(); } diff --git a/src/dbzero/object_model/dict/Dict.hpp b/src/dbzero/object_model/dict/Dict.hpp index b63aaa5f..aa80b27b 100644 --- a/src/dbzero/object_model/dict/Dict.hpp +++ b/src/dbzero/object_model/dict/Dict.hpp @@ -85,9 +85,8 @@ DB0_PACKED_END std::size_t size() const; void clear(); - + void commit() const; - void detach() const; void unrefMembers() const; diff --git a/src/dbzero/object_model/dict/DictIterator.cpp b/src/dbzero/object_model/dict/DictIterator.cpp index 714064aa..c8d81c1d 100644 --- a/src/dbzero/object_model/dict/DictIterator.cpp +++ b/src/dbzero/object_model/dict/DictIterator.cpp @@ -23,6 +23,7 @@ namespace db0::object_model void DictIterator::setJoinIterator() { + assureAttached(); if (m_iterator != m_collection->end()) { auto [key, address] = *m_iterator; m_current_hash = key; @@ -38,6 +39,7 @@ namespace db0::object_model void DictIterator::iterNext() { + assureAttached(); ++m_join_iterator; if (m_join_iterator.is_end()) { ++m_iterator; @@ -49,6 +51,7 @@ namespace db0::object_model DictIterator::DictItem DictIterator::nextItem() { + assureAttached(); auto fixture = m_collection->getFixture(); auto [key, value] = *m_join_iterator; @@ -61,14 +64,16 @@ namespace db0::object_model DictIterator::ObjectSharedPtr DictIterator::nextValue() { + assureAttached(); auto value = (*m_join_iterator).m_second; iterNext(); auto fixture = m_collection->getFixture(); return unloadMember(fixture, value, 0, m_member_flags); } - + DictIterator::ObjectSharedPtr DictIterator::nextKey() { + assureAttached(); auto key = (*m_join_iterator).m_first; iterNext(); auto fixture = m_collection->getFixture(); diff --git a/src/dbzero/object_model/dict/DictIterator.hpp b/src/dbzero/object_model/dict/DictIterator.hpp index 28aab9c3..b0b6e941 100644 --- a/src/dbzero/object_model/dict/DictIterator.hpp +++ b/src/dbzero/object_model/dict/DictIterator.hpp @@ -35,7 +35,7 @@ namespace db0::object_model ObjectSharedPtr nextKey(); // Restore the iterator after related collection was modified - void restore(); + void restore() override; protected: friend class Dict; diff --git a/src/dbzero/object_model/iterators/BaseIterator.hpp b/src/dbzero/object_model/iterators/BaseIterator.hpp index cafa269c..900fdc75 100644 --- a/src/dbzero/object_model/iterators/BaseIterator.hpp +++ b/src/dbzero/object_model/iterators/BaseIterator.hpp @@ -24,6 +24,7 @@ namespace db0::object_model using ObjectPtr = typename LangToolkit::ObjectPtr; using ObjectSharedPtr = typename LangToolkit::ObjectSharedPtr; using IteratorT = typename CollectionT::const_iterator; + virtual ObjectSharedPtr next() = 0; BaseIterator(IteratorT iterator, const CollectionT *ptr, ObjectPtr lang_collection_ptr) @@ -34,17 +35,33 @@ namespace db0::object_model { } - inline bool operator==(const ThisType &other) const { + bool operator==(const ThisType &other) const + { + assureAttached(); + other.assureAttached(); return m_iterator == other.m_iterator; } - inline bool operator!=(const ThisType &other) const { + bool operator!=(const ThisType &other) const + { + assureAttached(); + other.assureAttached(); return !(m_iterator == other.m_iterator); } - bool is_end() const { + bool is_end() const + { + assureAttached(); return m_iterator.is_end(); } + + void detach() const + { + // NOTE: this needs to be reimplemented to save key + iterator invalidation + m_detached = true; + } + + virtual void restore() = 0; protected: IteratorT m_iterator; @@ -53,6 +70,15 @@ namespace db0::object_model ObjectSharedPtr m_lang_collection_shared_ptr; // member access flags (e.g. no_cache) const AccessFlags m_member_flags; + mutable bool m_detached = false; + + void assureAttached() const + { + if (m_detached) { + const_cast(this)->restore(); + m_detached = false; + } + } }; } \ No newline at end of file diff --git a/src/dbzero/object_model/list/List.cpp b/src/dbzero/object_model/list/List.cpp index a0f0d3fa..ea9591af 100644 --- a/src/dbzero/object_model/list/List.cpp +++ b/src/dbzero/object_model/list/List.cpp @@ -221,4 +221,13 @@ namespace db0::object_model }); } + void List::detach() const + { + // detach object and the associated iterators + m_iterators.forEach([](ListIterator &iter) { + iter.detach(); + }); + super_t::detach(); + } + } diff --git a/src/dbzero/object_model/list/List.hpp b/src/dbzero/object_model/list/List.hpp index 3778ab3e..8c706221 100644 --- a/src/dbzero/object_model/list/List.hpp +++ b/src/dbzero/object_model/list/List.hpp @@ -73,6 +73,8 @@ namespace db0::object_model std::shared_ptr getIterator(ObjectPtr lang_list) const; + void detach() const; + private: // the associated iterator // which must be invalidated / refreshed on any collection modification diff --git a/src/dbzero/object_model/list/ListIterator.cpp b/src/dbzero/object_model/list/ListIterator.cpp index a47b8feb..0cd63f8a 100644 --- a/src/dbzero/object_model/list/ListIterator.cpp +++ b/src/dbzero/object_model/list/ListIterator.cpp @@ -15,6 +15,7 @@ namespace db0::object_model ListIterator::ObjectSharedPtr ListIterator::next() { + assureAttached(); auto fixture = m_collection->getFixture(); auto [storage_class, value] = *m_iterator; ++m_iterator; @@ -28,5 +29,5 @@ namespace db0::object_model // NOTE: may set the iterator as end m_iterator = this->m_collection->begin(m_index); } - + } \ No newline at end of file diff --git a/src/dbzero/object_model/list/ListIterator.hpp b/src/dbzero/object_model/list/ListIterator.hpp index dec70978..45a845aa 100644 --- a/src/dbzero/object_model/list/ListIterator.hpp +++ b/src/dbzero/object_model/list/ListIterator.hpp @@ -17,10 +17,10 @@ namespace db0::object_model { public: ObjectSharedPtr next() override; - + // try restoring the iterator after the related collection is modified // NOTE: may render the iterator as end - void restore(); + void restore() override; protected: friend class List; diff --git a/src/dbzero/object_model/set/Set.cpp b/src/dbzero/object_model/set/Set.cpp index da586a2a..3ff5b0b3 100644 --- a/src/dbzero/object_model/set/Set.cpp +++ b/src/dbzero/object_model/set/Set.cpp @@ -291,6 +291,10 @@ namespace db0::object_model // FIXME: can be removed when GC0 calls commit-op commit(); m_index.detach(); + // detach all associated iterators + m_iterators.forEach([](SetIterator &iter) { + iter.detach(); + }); super_t::detach(); } @@ -324,7 +328,7 @@ namespace db0::object_model iter.restore(); }); } - + Set::const_iterator Set::find(std::uint64_t key_hash) const { return m_index.find(key_hash); } diff --git a/src/dbzero/object_model/set/Set.hpp b/src/dbzero/object_model/set/Set.hpp index a61ddeaa..6f0f2456 100644 --- a/src/dbzero/object_model/set/Set.hpp +++ b/src/dbzero/object_model/set/Set.hpp @@ -79,9 +79,8 @@ DB0_PACKED_END void moveTo(db0::swine_ptr &); std::size_t size() const; - + void commit() const; - void detach() const; // drop underlying dbzero representation diff --git a/src/dbzero/object_model/set/SetIterator.cpp b/src/dbzero/object_model/set/SetIterator.cpp index b7d22154..4354beb8 100644 --- a/src/dbzero/object_model/set/SetIterator.cpp +++ b/src/dbzero/object_model/set/SetIterator.cpp @@ -17,6 +17,7 @@ namespace db0::object_model void SetIterator::setJoinIterator() { + assureAttached(); if (m_iterator != m_collection->end()) { auto [key, address] = *m_iterator; m_current_hash = key; @@ -32,6 +33,7 @@ namespace db0::object_model SetIterator::ObjectSharedPtr SetIterator::next() { + assureAttached(); auto fixture = m_collection->getFixture(); auto item = *m_join_iterator; auto [storage_class, value] = item; @@ -70,5 +72,5 @@ namespace db0::object_model setJoinIterator(); } } - + } \ No newline at end of file diff --git a/src/dbzero/object_model/set/SetIterator.hpp b/src/dbzero/object_model/set/SetIterator.hpp index a416efd7..2ded3f9f 100644 --- a/src/dbzero/object_model/set/SetIterator.hpp +++ b/src/dbzero/object_model/set/SetIterator.hpp @@ -21,7 +21,8 @@ namespace db0::object_model protected: friend class Set; SetIterator(Set::const_iterator iterator, const Set * ptr, ObjectPtr lang_set_ptr); - void restore(); + + void restore() override; private: SetIndex m_index; diff --git a/src/dbzero/object_model/tuple/TupleIterator.cpp b/src/dbzero/object_model/tuple/TupleIterator.cpp index 3bfc5e8c..cabf009d 100644 --- a/src/dbzero/object_model/tuple/TupleIterator.cpp +++ b/src/dbzero/object_model/tuple/TupleIterator.cpp @@ -21,7 +21,8 @@ namespace db0::object_model return unloadMember(fixture, storage_class, value, 0, m_member_flags); } - bool TupleIterator::is_end() const { + bool TupleIterator::is_end() const + { return m_iterator == m_collection->getData()->items().end(); } diff --git a/src/dbzero/object_model/tuple/TupleIterator.hpp b/src/dbzero/object_model/tuple/TupleIterator.hpp index 1c386f9a..b40453ee 100644 --- a/src/dbzero/object_model/tuple/TupleIterator.hpp +++ b/src/dbzero/object_model/tuple/TupleIterator.hpp @@ -16,9 +16,13 @@ namespace db0::object_model class TupleIterator : public BaseIterator { public: - ObjectSharedPtr next() override; + ObjectSharedPtr next() override; bool is_end() const; + + void restore() override { + // does nothing, tuple is immutable + } protected: friend class Tuple;