From 0ded97ba5be6cba2736287ba0300813c5654c841 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 30 Jun 2026 16:13:11 -0400 Subject: [PATCH] Make the getKeys() allocation more robust - Make it very unlikely that the allocation loop will ever need to run more than once. Recover gracefully if it does. - Prevent livelock by limiting the total possible number of iterations. - If this ever happens, throw our metaphorical hands in the air, and allocate under lock. - Pad the size before allocating to give the cache a little room to grow while not holding the lock. - Pad by less on each iteration. - Assert that no more than two iterations occur. Even two is probably overkill, but this allows for rare edge case scenarios. e.g. The cache is very small, the allocation takes a long time (which is contradictory) and a handful of items get added, overcoming the padding. --- include/xrpl/basics/TaggedCache.ipp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/include/xrpl/basics/TaggedCache.ipp b/include/xrpl/basics/TaggedCache.ipp index 4b7e1fe5fb..8ba50c8f87 100644 --- a/include/xrpl/basics/TaggedCache.ipp +++ b/include/xrpl/basics/TaggedCache.ipp @@ -596,11 +596,33 @@ TaggedCache v; { + // Keep track of how many iterations are needed. Exit the loop if the number of retries gets + // absurd. (Note that if this somehow ever happens, one more allocation will be done under + // lock, which is undesirable, but really should be almost impossible. Also, assert that + // there were fewer than 3 needed after the loop, because in a normal operating environment, + // even 2 is going to be unusual, and 3 shouldn't be needed. + std::size_t allocationIterations = 0; std::unique_lock lock(mutex_); - for (auto size = cache_.size(); v.capacity() < size; size = cache_.size()) + for (auto size = cache_.size(); v.capacity() < size && allocationIterations < 20; + size = cache_.size()) { ScopeUnlock const unlock(lock); + // Allocate the current size plus a little extra, in case the cache grows while + // allocating. Each time another allocation is needed, the extra also gets bigger until + // it ultimately doubles the size + 1. + size += (size >> (4 - std::min(allocationIterations, 4ul))) + 1; v.reserve(size); + ++allocationIterations; + } + XRPL_ASSERT( + allocationIterations < 3, + "xrpl::TaggedCache::getKeys(): limited allocation iterations"); + if (v.capacity() < cache_.size()) + { + // LCOV_EXCL_START + UNREACHABLE("xrpl::TaggedCache::getKeys(): failed to allocate sufficient capacity"); + v.reserve(cache_.size()); + // LCOV_EXCL_STOP } XRPL_ASSERT(lock.owns_lock(), "xrpl::TaggedCache::getKeys(): owns lock"); XRPL_ASSERT(