From 64b0bf964137c3752f97bed729af3ca1591414fe Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Thu, 12 Feb 2026 17:04:40 +0000 Subject: [PATCH] concurrency issues solved --- include/xrpl/ledger/LedgerIndexMap.h | 42 ++++-------------- src/tests/libxrpl/ledger/LedgerIndexMap.cpp | 48 +++++++++++---------- src/xrpld/app/ledger/LedgerHistory.cpp | 4 +- 3 files changed, 37 insertions(+), 57 deletions(-) diff --git a/include/xrpl/ledger/LedgerIndexMap.h b/include/xrpl/ledger/LedgerIndexMap.h index 6b4d5b19cd..e4d3e32b51 100644 --- a/include/xrpl/ledger/LedgerIndexMap.h +++ b/include/xrpl/ledger/LedgerIndexMap.h @@ -2,7 +2,9 @@ #include #include +#include #include +#include namespace xrpl { @@ -23,45 +25,19 @@ public: LedgerIndexMap& operator=(LedgerIndexMap&&) = delete; - Mapped& - operator[](Key const& k) - { - std::lock_guard lock(mutex_); - return data_[k]; - } - - Mapped& - operator[](Key&& k) - { - std::lock_guard lock(mutex_); - return data_[std::move(k)]; - } - - [[nodiscard]] Mapped* - get(Key const& k) - { - std::lock_guard lock(mutex_); - auto it = data_.find(k); - return it == data_.end() ? nullptr : &it->second; - } - - [[nodiscard]] Mapped const* + [[nodiscard]] std::optional get(Key const& k) const { std::lock_guard lock(mutex_); auto it = data_.find(k); - return it == data_.end() ? nullptr : &it->second; + return it == data_.end() ? std::nullopt : std::optional{it->second}; } - template - Mapped& - put(Key const& k, Args&&... args) + bool + put(Key const& k, Mapped value) { std::lock_guard lock(mutex_); - auto [it, inserted] = data_.try_emplace(k, std::forward(args)...); - if (!inserted) - it->second = Mapped(std::forward(args)...); - return it->second; + return data_.insert_or_assign(k, std::move(value)).second; } bool @@ -72,14 +48,14 @@ public: } std::size_t - size() const noexcept + size() const { std::lock_guard lock(mutex_); return data_.size(); } bool - empty() const noexcept + empty() const { std::lock_guard lock(mutex_); return data_.empty(); diff --git a/src/tests/libxrpl/ledger/LedgerIndexMap.cpp b/src/tests/libxrpl/ledger/LedgerIndexMap.cpp index 9d0383544e..559de33561 100644 --- a/src/tests/libxrpl/ledger/LedgerIndexMap.cpp +++ b/src/tests/libxrpl/ledger/LedgerIndexMap.cpp @@ -15,38 +15,40 @@ TEST(LedgerIndexMap, DefaultEmpty) TestMap m; EXPECT_EQ(m.size(), 0); EXPECT_TRUE(m.empty()); - EXPECT_EQ(m.get(42), nullptr); + EXPECT_FALSE(m.get(42).has_value()); EXPECT_FALSE(m.contains(42)); } -TEST(LedgerIndexMap, OperatorBracketInsertsDefault) +TEST(LedgerIndexMap, PutInsertsValue) { TestMap m; - auto& v = m[10]; + bool inserted = m.put(10, ""); + EXPECT_TRUE(inserted); EXPECT_EQ(m.size(), 1); EXPECT_TRUE(m.contains(10)); - EXPECT_TRUE(v.empty()); + auto got = m.get(10); + ASSERT_TRUE(got.has_value()); + EXPECT_TRUE(got->empty()); } -TEST(LedgerIndexMap, OperatorBracketRvalueKey) +TEST(LedgerIndexMap, PutWithValue) { TestMap m; - int k = 7; - auto& v1 = m[std::move(k)]; - v1 = "seven"; + bool inserted = m.put(7, "seven"); + EXPECT_TRUE(inserted); EXPECT_EQ(m.size(), 1); - auto* got = m.get(7); - ASSERT_NE(got, nullptr); + auto got = m.get(7); + ASSERT_TRUE(got.has_value()); EXPECT_EQ(*got, "seven"); } TEST(LedgerIndexMap, InsertThroughPut) { TestMap m; - auto& v = m.put(20, "twenty"); - EXPECT_EQ(v, "twenty"); - auto* got = m.get(20); - ASSERT_NE(got, nullptr); + bool inserted = m.put(20, "twenty"); + EXPECT_TRUE(inserted); + auto got = m.get(20); + ASSERT_TRUE(got.has_value()); EXPECT_EQ(*got, "twenty"); EXPECT_EQ(m.size(), 1); } @@ -54,12 +56,14 @@ TEST(LedgerIndexMap, InsertThroughPut) TEST(LedgerIndexMap, OverwriteExistingKeyWithPut) { TestMap m; - m.put(5, "five"); + bool inserted1 = m.put(5, "five"); + EXPECT_TRUE(inserted1); EXPECT_EQ(m.size(), 1); - m.put(5, "FIVE"); + bool inserted2 = m.put(5, "FIVE"); + EXPECT_FALSE(inserted2); // Not a new insertion, it's an update EXPECT_EQ(m.size(), 1); - auto* got = m.get(5); - ASSERT_NE(got, nullptr); + auto got = m.get(5); + ASSERT_TRUE(got.has_value()); EXPECT_EQ(*got, "FIVE"); } @@ -67,8 +71,8 @@ TEST(LedgerIndexMap, OnceFoundOneNotFound) { TestMap m; m.put(1, "one"); - EXPECT_NE(m.get(1), nullptr); - EXPECT_EQ(m.get(2), nullptr); + EXPECT_TRUE(m.get(1).has_value()); + EXPECT_FALSE(m.get(2).has_value()); } TEST(LedgerIndexMap, TryEraseBeforeNothingToDo) @@ -174,8 +178,8 @@ TEST(LedgerIndexMap, RehashDoesNotLoseEntries) for (int k = 0; k < 16; ++k) { - auto* v = m.get(k); - ASSERT_NE(v, nullptr); + auto v = m.get(k); + ASSERT_TRUE(v.has_value()); EXPECT_EQ(*v, "v" + std::to_string(k)); } diff --git a/src/xrpld/app/ledger/LedgerHistory.cpp b/src/xrpld/app/ledger/LedgerHistory.cpp index c2877fb794..233e3093b2 100644 --- a/src/xrpld/app/ledger/LedgerHistory.cpp +++ b/src/xrpld/app/ledger/LedgerHistory.cpp @@ -34,7 +34,7 @@ LedgerHistory::insert(std::shared_ptr const& ledger, bool validate bool const alreadyHad = m_ledgers_by_hash.canonicalize_replace_cache(ledger->header().hash, ledger); if (validated) - mLedgersByIndex[ledger->header().seq] = ledger->header().hash; + mLedgersByIndex.put(ledger->header().seq, ledger->header().hash); return alreadyHad; } @@ -67,7 +67,7 @@ LedgerHistory::getLedgerBySeq(LedgerIndex index) // Add this ledger to the local tracking by index XRPL_ASSERT(ret->isImmutable(), "xrpl::LedgerHistory::getLedgerBySeq : immutable result ledger"); m_ledgers_by_hash.canonicalize_replace_client(ret->header().hash, ret); - mLedgersByIndex[ret->header().seq] = ret->header().hash; + mLedgersByIndex.put(ret->header().seq, ret->header().hash); return (ret->header().seq == index) ? ret : nullptr; } }