Skip to content

bugfixes #780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wskozlowski merged 1 commit into from
Jun 2, 2026
Merged

bugfixes #780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions python_tests/test_atomic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,9 @@ def test_leaked_find_iterator_advanced_inside_canceled_atomic_repositions_after_
run_pytest_child(
"python_tests/test_atomic.py::test_leaked_find_iterator_advanced_inside_canceled_atomic_repositions_after_rollback_child",
env_flag="DB0_ATOMIC_LEAKED_FIND_ITERATOR_CHILD",
timeout=30,
failure_label="atomic leaked find iterator child",
pytest_args=("-o", "faulthandler_timeout=10"),
)


Expand All @@ -1122,6 +1124,24 @@ def test_leaked_find_iterator_advanced_inside_canceled_atomic_repositions_after_
reason="executed by test_leaked_find_iterator_advanced_inside_canceled_atomic_repositions_after_rollback",
)
def test_leaked_find_iterator_advanced_inside_canceled_atomic_repositions_after_rollback_child(db0_no_autocommit):
def advance(iterator, count):
values = []
for _ in range(count):
try:
values.append(next(iterator).value)
except StopIteration:
break
return values

def assert_no_rolled_back_values(values, iteration):
assert not any(
isinstance(value, tuple)
and len(value) >= 2
and value[0] == "rolled-back"
and value[1] == iteration
for value in values
)

first = MemoTestClass("first")
db0.commit()

Expand All @@ -1143,6 +1163,38 @@ def test_leaked_find_iterator_advanced_inside_canceled_atomic_repositions_after_
with pytest.raises(StopIteration):
next(iterator)

rng = random.Random(0xDB0F17D)
leaked_iterators = [iterator]
seeds = [MemoTestClass(("seed", index)) for index in range(32)]
db0.commit()

iteration_count = int(os.environ.get("DB0_ATOMIC_LEAKED_FIND_ITERATOR_ITERATIONS", "100"))
for iteration in range(iteration_count):
iterator = iter(db0.find(MemoTestClass))
advance(iterator, rng.randrange(1, len(seeds) + 5))

with db0.atomic() as atomic:
for offset in range(rng.randrange(1, 8)):
MemoTestClass(("rolled-back", iteration, offset))

values = advance(iterator, rng.randrange(1, len(seeds) + 8))
assert_no_rolled_back_values(values, iteration)

if rng.random() < 0.5:
list(db0.find(MemoTestClass))

atomic.cancel()

values = advance(iterator, rng.randrange(1, 16))
assert_no_rolled_back_values(values, iteration)

leaked_iterators.append(iterator)
if len(leaked_iterators) > 64:
del leaked_iterators[:32]

if iteration % 25 == 0:
gc.collect()


@pytest.mark.skip(reason=ATOMIC_LEAKED_ITERATOR_REPRO_SKIP)
def test_leaked_iterator_advanced_inside_canceled_atomic_repositions_after_rollback(run_pytest_child):
Expand Down
6 changes: 4 additions & 2 deletions src/dbzero/core/collections/full_text/FT_BaseIndex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ namespace db0
}
return std::unique_ptr<FT_Iterator<KeyT> >(
new FT_IndexIterator<ListT, KeyT, IndexKeyT>(
*inverted_list_ptr, direction, key, std::move(index_key_sequence))
*inverted_list_ptr, direction, key, std::move(index_key_sequence),
[this, key]() { return this->tryGetExistingInvertedList(key); })
);
}

Expand All @@ -79,7 +80,8 @@ namespace db0
// key inverted index
factory.add(std::unique_ptr<FT_Iterator<KeyT> >(
new FT_IndexIterator<ListT, KeyT, IndexKeyT>(
*inverted_list_ptr, -1, key, std::move(index_key_sequence)))
*inverted_list_ptr, -1, key, std::move(index_key_sequence),
[this, key]() { return this->tryGetExistingInvertedList(key); }))
);
return true;
}
Expand Down
6 changes: 4 additions & 2 deletions src/dbzero/core/collections/full_text/FT_BaseIndex.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,11 @@ namespace db0
// build query tree
for (const auto &inverted_list: inverted_lists) {
// key inverted index
auto key = inverted_list.first;
factory.add(std::unique_ptr<FT_Iterator<KeyT> >(
new FT_IndexIterator<ListT, KeyT>(
*inverted_list.second, -1, inverted_list.first, {}))
new FT_IndexIterator<ListT, KeyT, IndexKeyT>(
*inverted_list.second, -1, key, {},
[this, key]() { return this->tryGetExistingInvertedList(key); }))
);
}
return result;
Expand Down
96 changes: 62 additions & 34 deletions src/dbzero/core/collections/full_text/FT_IndexIterator.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <memory>
#include <optional>
#include <vector>
#include <functional>

namespace db0

Expand All @@ -27,6 +28,8 @@ namespace db0
using self_t = FT_IndexIterator<bindex_t, key_t, IndexKeyT>;
using super_t = FT_Iterator<key_t>;
using iterator = typename bindex_t::joinable_const_iterator;
using data_ptr = std::shared_ptr<const bindex_t>;
using data_resolver = std::function<data_ptr()>;

// data is the live b-index view used to build the native iterator.
// detach/reattach rebuilds only the native iterator state from this view,
Expand All @@ -38,7 +41,7 @@ namespace db0
// tag iterators it stores the root-to-leaf tag key path so deserialize
// can traverse from the root TagIndex instead of persisting child index addresses.
FT_IndexIterator(const bindex_t &data, int direction, std::optional<IndexKeyT> index_key = {},
std::vector<IndexKeyT> &&index_key_sequence = {});
std::vector<IndexKeyT> &&index_key_sequence = {}, data_resolver resolver = {});

FT_IndexIterator(bindex_t &&data, int direction, std::optional<IndexKeyT> index_key = {},
std::vector<IndexKeyT> &&index_key_sequence = {});
Expand All @@ -47,7 +50,8 @@ namespace db0
* Construct over already initialized simple iterator
*/
FT_IndexIterator(const bindex_t &data, int direction, const iterator &it,
std::optional<IndexKeyT> index_key = {}, std::vector<IndexKeyT> &&index_key_sequence = {});
std::optional<IndexKeyT> index_key = {}, std::vector<IndexKeyT> &&index_key_sequence = {},
data_resolver resolver = {});

virtual ~FT_IndexIterator() = default;

Expand Down Expand Up @@ -93,8 +97,15 @@ namespace db0
void getSignature(std::vector<std::byte> &) const override;

protected:
std::unique_ptr<bindex_t> m_owned_data;
const bindex_t *m_data = nullptr;
// Current b-index view used by m_iterator. Resolver-backed iterators clear it
// on detach so reattachment cannot accidentally reuse a stale MorphingBIndex
// view. Temporary in-memory indexes own the moved b-index through this pointer.
data_ptr m_data;
// Optional reattachment hook. Persistent tag iterators use it to re-resolve
// the current inverted-list b-index after rollback/morphing may have changed
// the underlying MorphingBIndex address/type. Temporary indexes resolve to
// their owned m_data; plain borrowed indexes leave it empty.
data_resolver m_data_resolver;
const int m_direction;
// underlying native iterator (joinable_const_iterator)
mutable iterator m_iterator;
Expand All @@ -108,11 +119,14 @@ namespace db0
const std::vector<IndexKeyT> m_index_key_sequence;

FT_IndexIterator(std::uint64_t uid, const bindex_t &data, int direction,
std::optional<IndexKeyT> index_key = {}, std::vector<IndexKeyT> &&index_key_sequence = {});
std::optional<IndexKeyT> index_key = {}, std::vector<IndexKeyT> &&index_key_sequence = {},
data_resolver resolver = {});

FT_IndexIterator(std::uint64_t uid, bindex_t &&data, int direction,
std::optional<IndexKeyT> index_key = {}, std::vector<IndexKeyT> &&index_key_sequence = {});

static data_ptr borrowData(const bindex_t &data);

/**
* Get valid iterator after detach
* @return
Expand All @@ -132,10 +146,12 @@ namespace db0

template <typename bindex_t, typename key_t, typename IndexKeyT>
FT_IndexIterator<bindex_t, key_t, IndexKeyT>::FT_IndexIterator(const bindex_t &data, int direction,
std::optional<IndexKeyT> index_key, std::vector<IndexKeyT> &&index_key_sequence)
: m_data(&data)
std::optional<IndexKeyT> index_key, std::vector<IndexKeyT> &&index_key_sequence,
data_resolver resolver)
: m_data(borrowData(data))
, m_data_resolver(std::move(resolver))
, m_direction(direction)
, m_iterator(m_data->beginJoin(direction))
, m_iterator(data.beginJoin(direction))
, m_index_key(index_key)
, m_index_key_sequence(std::move(index_key_sequence))
{
Expand All @@ -144,8 +160,8 @@ namespace db0
template <typename bindex_t, typename key_t, typename IndexKeyT>
FT_IndexIterator<bindex_t, key_t, IndexKeyT>::FT_IndexIterator(bindex_t &&data, int direction,
std::optional<IndexKeyT> index_key, std::vector<IndexKeyT> &&index_key_sequence)
: m_owned_data(std::make_unique<bindex_t>(std::move(data)))
, m_data(m_owned_data.get())
: m_data(std::make_shared<bindex_t>(std::move(data)))
, m_data_resolver([data = m_data]() { return data; })
, m_direction(direction)
, m_iterator(m_data->beginJoin(direction))
, m_index_key(index_key)
Expand All @@ -155,8 +171,10 @@ namespace db0

template <typename bindex_t, typename key_t, typename IndexKeyT>
FT_IndexIterator<bindex_t, key_t, IndexKeyT>::FT_IndexIterator(const bindex_t &data, int direction, const iterator &it,
std::optional<IndexKeyT> index_key, std::vector<IndexKeyT> &&index_key_sequence)
: m_data(&data)
std::optional<IndexKeyT> index_key, std::vector<IndexKeyT> &&index_key_sequence,
data_resolver resolver)
: m_data(borrowData(data))
, m_data_resolver(std::move(resolver))
, m_direction(direction)
, m_iterator(it)
, m_index_key(index_key)
Expand All @@ -166,11 +184,13 @@ namespace db0

template <typename bindex_t, typename key_t, typename IndexKeyT>
FT_IndexIterator<bindex_t, key_t, IndexKeyT>::FT_IndexIterator(std::uint64_t uid, const bindex_t &data, int direction,
std::optional<IndexKeyT> index_key, std::vector<IndexKeyT> &&index_key_sequence)
std::optional<IndexKeyT> index_key, std::vector<IndexKeyT> &&index_key_sequence,
data_resolver resolver)
: FT_Iterator<key_t>(uid)
, m_data(&data)
, m_data(borrowData(data))
, m_data_resolver(std::move(resolver))
, m_direction(direction)
, m_iterator(m_data->beginJoin(direction))
, m_iterator(data.beginJoin(direction))
, m_index_key(index_key)
, m_index_key_sequence(std::move(index_key_sequence))
{
Expand All @@ -180,8 +200,8 @@ namespace db0
FT_IndexIterator<bindex_t, key_t, IndexKeyT>::FT_IndexIterator(std::uint64_t uid, bindex_t &&data, int direction,
std::optional<IndexKeyT> index_key, std::vector<IndexKeyT> &&index_key_sequence)
: FT_Iterator<key_t>(uid)
, m_owned_data(std::make_unique<bindex_t>(std::move(data)))
, m_data(m_owned_data.get())
, m_data(std::make_shared<bindex_t>(std::move(data)))
, m_data_resolver([data = m_data]() { return data; })
, m_direction(direction)
, m_iterator(m_data->beginJoin(direction))
, m_index_key(index_key)
Expand Down Expand Up @@ -248,15 +268,9 @@ namespace db0

template <typename bindex_t, typename key_t, typename IndexKeyT>
std::unique_ptr<FT_Iterator<key_t> > FT_IndexIterator<bindex_t, key_t, IndexKeyT>::beginTyped(int direction) const {
if (m_owned_data) {
return std::unique_ptr<FT_Iterator<key_t> >(
new FT_IndexIterator(this->m_uid, bindex_t(*m_data), direction, this->m_index_key,
std::vector<IndexKeyT>(this->m_index_key_sequence))
);
}
return std::unique_ptr<FT_Iterator<key_t> >(
new FT_IndexIterator(this->m_uid, *m_data, direction, this->m_index_key,
std::vector<IndexKeyT>(this->m_index_key_sequence))
std::vector<IndexKeyT>(this->m_index_key_sequence), this->m_data_resolver)
);
}

Expand Down Expand Up @@ -298,6 +312,9 @@ namespace db0
this->m_detach_key = *(this->m_iterator);
}
this->m_iterator.reset();
if (this->m_data_resolver) {
this->m_data.reset();
}
this->m_is_detached = true;
}
}
Expand All @@ -306,26 +323,37 @@ namespace db0
typename FT_IndexIterator<bindex_t, key_t, IndexKeyT>::iterator &FT_IndexIterator<bindex_t, key_t, IndexKeyT>::getIterator()
{
if (m_is_detached) {
try {
m_force_end = false;
m_iterator = m_data->beginJoin(m_direction);
if (m_detach_key) {
if (!m_iterator.join(*m_detach_key, m_direction)) {
m_iterator.stop();
m_force_end = true;
}
} else {
m_force_end = false;
if (m_data_resolver) {
m_data = m_data_resolver();
}
if (!m_data) {
m_force_end = true;
m_is_detached = false;
return m_iterator;
}
m_iterator = m_data->beginJoin(m_direction);
if (m_detach_key) {
if (!m_iterator.join(*m_detach_key, m_direction)) {
m_iterator.stop();
m_force_end = true;
}
} catch (...) {
} else {
m_iterator.stop();
m_force_end = true;
}
m_is_detached = false;
}
return m_iterator;
}

template <typename bindex_t, typename key_t, typename IndexKeyT>
typename FT_IndexIterator<bindex_t, key_t, IndexKeyT>::data_ptr
FT_IndexIterator<bindex_t, key_t, IndexKeyT>::borrowData(const bindex_t &data)
{
return data_ptr(&data, [](const bindex_t *) {});
}

template <typename bindex_t, typename key_t, typename IndexKeyT>
const typename FT_IndexIterator<bindex_t, key_t, IndexKeyT>::iterator &FT_IndexIterator<bindex_t, key_t, IndexKeyT>::getIterator() const
{
Expand Down
5 changes: 1 addition & 4 deletions src/dbzero/core/collections/range_tree/FT_BoundIterator.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,7 @@ namespace db0
template <typename KeyT, typename ValueT, typename IndexT>
std::unique_ptr<FT_Iterator<ValueT> > FT_BoundIterator<KeyT, ValueT, IndexT>::beginTyped(int direction) const
{
if (super_t::m_owned_data) {
return std::unique_ptr<FT_Iterator<ValueT> >(new self_t(IndexT(*super_t::m_data), direction, m_key_range));
}
return std::unique_ptr<FT_Iterator<ValueT> >(new self_t(*super_t::m_data, direction, m_key_range));
return std::unique_ptr<FT_Iterator<ValueT> >(new self_t(IndexT(*super_t::m_data), direction, m_key_range));
}

}
Loading