diff --git a/indra/llmessage/llpacketring.cpp b/indra/llmessage/llpacketring.cpp index bade413e61..0c02ca1453 100644 --- a/indra/llmessage/llpacketring.cpp +++ b/indra/llmessage/llpacketring.cpp @@ -29,6 +29,7 @@ #include "llpacketring.h" #include "llerror.h" +#include "llframetimer.h" constexpr S16 MAX_BUFFER_RING_SIZE = 8192; @@ -59,35 +60,39 @@ LLPacketRing::~LLPacketRing() mHeadIndex = 0; } -void LLPacketRing::pushPacket(const LLPacketBuffer& packet) +bool LLPacketRing::hasRoom() const { - S16 ring_size = (S16)mRing.size(); - if (mNumBufferedPackets >= ring_size && ring_size < MAX_BUFFER_RING_SIZE) + return mNumBufferedPackets < (S16)mRing.size() || (S16)mRing.size() < MAX_BUFFER_RING_SIZE; +} + +bool LLPacketRing::pushPacket(const LLPacketBuffer& packet) +{ + if (mNumBufferedPackets >= (S16)mRing.size() && !expandRing()) { - expandRing(); - ring_size = (S16)mRing.size(); + // A flood that fills the ring leaves a packet out with every one + // that comes in, so it is told of a second at a time. + ++mLeftOutSinceWarning; + const F64 now = LLFrameTimer::getTotalSeconds(); + if (!mWarnedLeftOut || now - mLastLeftOutWarning >= 1.0) + { + LL_WARNS("PacketRing") << "buffer full at " << mNumBufferedPackets << " packets, " << mLeftOutSinceWarning + << " left out" << LL_ENDL; + mWarnedLeftOut = true; + mLastLeftOutWarning = now; + mLeftOutSinceWarning = 0; + } + return false; } + const S16 ring_size = (S16)mRing.size(); LLPacketBuffer* slot = mRing[mHeadIndex]; - S32 old_size = slot->getSize(); - *slot = packet; mHeadIndex = (mHeadIndex + 1) % ring_size; - if (mNumBufferedPackets < ring_size) - { - ++mNumBufferedPackets; - mNumBufferedBytes += packet.getSize(); - } - else - { - // Ring is at maximum capacity; oldest packet was overwritten. - // This is VERY BAD because we've already ACKed the packet we're loosing - // (if it was "reliable"). - LL_WARNS("PacketRing") << "buffer overflow at " << mNumBufferedPackets << " packets" << LL_ENDL; - mNumBufferedBytes += packet.getSize() - old_size; - } + ++mNumBufferedPackets; + mNumBufferedBytes += packet.getSize(); + return true; } bool LLPacketRing::popPacket(LLPacketBuffer& packet) diff --git a/indra/llmessage/llpacketring.h b/indra/llmessage/llpacketring.h index 5315e5a5bd..934abd0adc 100644 --- a/indra/llmessage/llpacketring.h +++ b/indra/llmessage/llpacketring.h @@ -8,9 +8,8 @@ * popPacket(). * * The ring starts at DEFAULT_BUFFER_RING_SIZE slots and grows in increments - * of BUFFER_RING_EXPANSION up to MAX_BUFFER_RING_SIZE. Once at the ceiling, - * pushPacket() silently overwrites the oldest queued packet to make room for - * the incoming one. + * of BUFFER_RING_EXPANSION up to MAX_BUFFER_RING_SIZE. Once full at the + * ceiling, pushPacket() refuses the incoming packet. * * $LicenseInfo:firstyear=2001&license=viewerlgpl$ * Second Life Viewer Source Code @@ -47,9 +46,14 @@ class LLPacketRing LLPacketRing(); ~LLPacketRing(); - // Copy 'packet' onto the tail of the ring, growing the ring or - // overwriting the oldest entry when the ring is at capacity. - void pushPacket(const LLPacketBuffer& packet); + // Copy 'packet' onto the tail of the ring, growing the ring when it is + // full. A ring full at its ceiling leaves the packet out and returns + // false: the oldest packet it holds may be a reliable one already acked, + // which its sender would never send again. + bool pushPacket(const LLPacketBuffer& packet); + + // Whether pushPacket() would take a packet now. + bool hasRoom() const; // Copy the head (oldest) packet into 'packet'. // Returns true if a packet was available, false if the ring was empty. @@ -69,4 +73,8 @@ class LLPacketRing S16 mHeadIndex { 0 }; S16 mNumBufferedPackets { 0 }; S32 mNumBufferedBytes { 0 }; + + S32 mLeftOutSinceWarning { 0 }; + F64 mLastLeftOutWarning { 0.0 }; + bool mWarnedLeftOut { false }; }; diff --git a/indra/llmessage/message.cpp b/indra/llmessage/message.cpp index c27cd9ed59..68ddb339fc 100644 --- a/indra/llmessage/message.cpp +++ b/indra/llmessage/message.cpp @@ -1159,31 +1159,6 @@ S32 LLMessageSystem::bufferInboundPacket() } } - if (cdp) - { - // ACK inbound reliable packet ASAP, so its sender stops resending - // it while it waits in the queue, whatever checkMessages() makes - // of it, which does not ack it again. - if ((data[0] & LL_RELIABLE_FLAG)) - { - cdp->collectRAck(recv_packet_id); - pkt.setAcked(true); - } - - // Check packet sequencing here, in true socket-arrival order, before - // this packet is sorted into the high/low priority inbound queue. - // Skip duplicates of reliable packets already handled, marked resent - // or not, same as checkMessages()/logValidMsg() would do further - // downstream. - bool recv_resent = (data[0] & LL_RESENT_FLAG) != 0; - bool recv_reliable = (data[0] & LL_RELIABLE_FLAG) != 0; - if (!recv_reliable || !cdp->checkDuplicate(recv_packet_id, recv_resent)) - { - cdp->checkPacketInID(recv_packet_id, recv_resent); - pkt.setPacketIDChecked(true); - } - } - auto enqueue = [this](const LLPacketBuffer& buffered) { if (isHighPriorityMessage(buffered)) @@ -1196,13 +1171,54 @@ S32 LLMessageSystem::bufferInboundPacket() } }; + auto take = [&](LLPacketBuffer& arrived) + { + // A queue full at its ceiling has no room for the packet, which is + // left out as the network might have dropped it: neither acked nor + // its number recorded, so that its sender sends it again. + const bool has_room = (isHighPriorityMessage(arrived) ? mHighPriorityInbound : mLowPriorityInbound).hasRoom(); + + if (cdp) + { + // Skip duplicates of reliable packets already handled, marked resent + // or not, same as checkMessages()/logValidMsg() would do further + // downstream. + const bool recv_resent = (data[0] & LL_RESENT_FLAG) != 0; + const bool recv_reliable = (data[0] & LL_RELIABLE_FLAG) != 0; + const bool duplicate = recv_reliable && cdp->checkDuplicate(recv_packet_id, recv_resent); + + // ACK inbound reliable packet ASAP, so its sender stops resending + // it while it waits in the queue, whatever checkMessages() makes + // of it, which does not ack it again. A copy of a packet already + // taken is acked with no room for it, as it would be with room. + if (recv_reliable && (has_room || duplicate)) + { + cdp->collectRAck(recv_packet_id); + arrived.setAcked(true); + } + + // Check packet sequencing here, in true socket-arrival order, before + // this packet is sorted into the high/low priority inbound queue. + if (has_room && !duplicate) + { + cdp->checkPacketInID(recv_packet_id, recv_resent); + arrived.setPacketIDChecked(true); + } + } + + enqueue(arrived); + }; + #if AL_NET_IMPAIRMENT std::vector released; mNetImpairment.advance(released); + // A packet held back arrives when it is released: checkMessages() acks + // it and checks its number as it reads it, and a queue with no room for + // it then leaves it out unacked. if (!mNetImpairment.hold(pkt)) { - enqueue(pkt); + take(pkt); } // Anything released rejoins *behind* the packet that just arrived, which is what puts it @@ -1212,7 +1228,7 @@ S32 LLMessageSystem::bufferInboundPacket() enqueue(late); } #else - enqueue(pkt); + take(pkt); #endif } diff --git a/indra/llmessage/tests/message_test.cpp b/indra/llmessage/tests/message_test.cpp index fd889d9ba9..3d2ceee1ab 100644 --- a/indra/llmessage/tests/message_test.cpp +++ b/indra/llmessage/tests/message_test.cpp @@ -36,6 +36,7 @@ #include "net.h" #include +#include #if !LL_WINDOWS #include @@ -67,6 +68,12 @@ namespace ll_test { return std::count(circuit.mAcks.begin(), circuit.mAcks.end(), id); } + + // The number the circuit takes the next packet in order to have. + static TPACKETID packetsInID(const LLCircuitData& circuit) + { + return circuit.mPacketsInID; + } }; } @@ -752,5 +759,113 @@ namespace } CHECK(ll_test::CircuitProbe::acksWaiting(*circuit) == 5); } + + // The packets a full queue holds were acked as they came in, so taking + // the place of the oldest would lose a packet its sender never sends + // again. The packet the queue has no room for is left out unacked. + TEST_CASE_METHOD(LLMessageSystemTestData, "LLMessageSystem: a full inbound queue leaves a reliable packet out unacked", "[LLMessageSystem]") + { + const std::string template_path = mTestConfigDir + mSep + "message_template.msg"; + const TemplateFile message_template(template_path, DUPLICATE_TEST_TEMPLATE); + LLMessageSystem system(template_path, NET_USE_OS_ASSIGNED_PORT, 1, 0, 0, false, 5.f, 100.f); + REQUIRE(system.isOK()); + const UseMessageSystem use(system); + system.setHandlerFunc("DuplicateTest", handle_duplicate_test); + sDuplicateTestsHandled = 0; + + const SenderSocket sender; + REQUIRE(sender.mResult == 0); + const LLHost sender_host("127.0.0.1", sender.mPort); + system.enableCircuit(sender_host, true); + const LLCircuitData* circuit = system.mCircuitInfo.findCircuit(sender_host); + REQUIRE(circuit != nullptr); + const LLHost receiver("127.0.0.1", system.getListenPort()); + + // A packet numbered `id` carrying message number `message`. + using MessageNumber = std::array; + auto send = [&](U32 id, U8 flags, const MessageNumber& message) + { + const U8 packet[] = { flags, (U8)(id >> 24), (U8)(id >> 16), (U8)(id >> 8), (U8)id, 0, + message[0], message[1], message[2], message[3], 1, 2, 3, 4 }; + REQUIRE(send_packet(sender.mSocket, (const char*)packet, (int)sizeof(packet), receiver.getAddress(), receiver.getPort())); + }; + + // Waits until the system has left `packets` packets out in all. + auto drain_until_left_out = [&](S32 packets) + { + const F64 give_up = LLTimer::getTotalSeconds() + 1.0; + while (system.getTotalNumDroppedPackets() < packets && LLTimer::getTotalSeconds() < give_up) + { + system.drainUdpSocket(); + ms_sleep(1); + } + REQUIRE(system.getTotalNumDroppedPackets() == packets); + }; + + // Fills the ring `message` goes to up to its ceiling with reliable + // packets numbered from `first`, a few at a time so that the socket's + // buffer holds every one until it is queued, and sends one more, + // which is left out. Returns its number. + constexpr S32 FULL = 8192; + auto overflow = [&](U32 first, const MessageNumber& message) -> U32 + { + const size_t acks_before = ll_test::CircuitProbe::acksWaiting(*circuit); + U32 id = first; + while (system.getNumBufferedPackets() < FULL) + { + const S32 batch = llmin(128, FULL - system.getNumBufferedPackets()); + for (S32 i = 0; i < batch; ++i) + { + send(id++, LL_RELIABLE_FLAG, message); + } + queue_until(system, (S32)(id - first)); + REQUIRE(system.getNumBufferedPackets() == (S32)(id - first)); + } + REQUIRE(ll_test::CircuitProbe::acksWaiting(*circuit) == acks_before + FULL); + + const U32 left_out = id; + send(left_out, LL_RELIABLE_FLAG, message); + drain_until_left_out(1); + CHECK(system.getNumBufferedPackets() == FULL); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, left_out) == 0); + CHECK(ll_test::CircuitProbe::acksWaiting(*circuit) == acks_before + FULL); + UNSCOPED_INFO("nor is its number taken as come"); + CHECK(ll_test::CircuitProbe::packetsInID(*circuit) == left_out); + return left_out; + }; + + SECTION("the low priority ring") + { + // DuplicateTest, Low 1. + const MessageNumber duplicate_test = { 0xff, 0xff, 0x00, 0x01 }; + send(1, LL_RELIABLE_FLAG, duplicate_test); + read_until(system, 1); + REQUIRE(sDuplicateTestsHandled == 1); + const U32 left_out = overflow(2, duplicate_test); + + UNSCOPED_INFO("a copy of a packet already read is acked all the same, as one it would drop"); + send(1, LL_RELIABLE_FLAG | LL_RESENT_FLAG, duplicate_test); + drain_until_left_out(2); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, 1) == 2); + + UNSCOPED_INFO("every packet queued is read, the first among them"); + read_queued(system); + CHECK(sDuplicateTestsHandled == FULL + 1); + + UNSCOPED_INFO("and the one left out is taken when it comes again"); + send(left_out, LL_RELIABLE_FLAG | LL_RESENT_FLAG, duplicate_test); + read_until(system, FULL + 2); + CHECK(sDuplicateTestsHandled == FULL + 2); + CHECK(ll_test::CircuitProbe::acksWaitingFor(*circuit, left_out) == 1); + } + + SECTION("the high priority ring") + { + // A Medium message, whose bytes after the number are none that + // isHighPriorityMessage keeps in the low priority ring. + const MessageNumber medium = { 0xff, 0x01, 0x01, 0x01 }; + overflow(1, medium); + } + } } diff --git a/indra/newview/llviewerregion.cpp b/indra/newview/llviewerregion.cpp index 93a2be6c94..b964082245 100644 --- a/indra/newview/llviewerregion.cpp +++ b/indra/newview/llviewerregion.cpp @@ -2962,6 +2962,14 @@ LLViewerRegion::eCacheUpdateResult LLViewerRegion::cacheFullUpdate(LLDataPackerB LLViewerObject::unpackU32(&dp, local_id, "LocalID"); LLViewerObject::unpackU32(&dp, crc, "CRC"); + // Nor is one sent whole kept in it, to be let go before it is made: it is + // asked for again, to come once the cache has been removed. + if (cacheRemovalRequested()) + { + addCacheMissFull(local_id); + return CACHE_UPDATE_DUPE; + } + LLVOCacheEntry* entry = getCacheEntry(local_id, false); if (entry) @@ -3100,10 +3108,24 @@ void LLViewerRegion::addToCreatedList(U32 local_id) // Get data packer for this object, if we have cached data // AND the CRC matches. JC +bool LLViewerRegion::cacheRemovalRequested() const +{ + return LLVOCache::instanceExists() && LLVOCache::getInstance()->isRemovalRequested(mHandle); +} + bool LLViewerRegion::probeCache(U32 local_id, U32 crc, U32 flags, U8 &cache_miss_type) { //llassert(mCacheLoaded); This assert failes often, changing to early-out -- davep, 2010/10/18 + // An object a cache about to be removed holds would never be made from it, + // so it is asked for as the removed cache would have it asked for. + if (cacheRemovalRequested()) + { + addCacheMiss(local_id, CACHE_MISS_TYPE_TOTAL); + cache_miss_type = CACHE_MISS_TYPE_TOTAL; + return false; + } + LLVOCacheEntry* entry = getCacheEntry(local_id, false); if (entry) @@ -3256,7 +3278,15 @@ void LLViewerRegion::dumpCache() void LLViewerRegion::clearVOCacheFromMemory() { - mImpl->mCacheMap.clear(); + // The map holds the only references to its entries, and the sets, the + // waiting list, the cache's tree, the drawables' octree entries and the + // eviction walk's cursor hold bare pointers to them. + std::vector> released; + LLVOCacheEntry::releaseAll(mImpl->mCacheMap, { &mImpl->mActiveSet, &mImpl->mWaitingSet, &mImpl->mVisibleEntries }, + [this](LLVOCacheEntry* entry) { removeFromVOCacheTree(entry); }, + LLAppViewer::instance()->isQuitting() ? released : sRegionCacheCleanup); + mImpl->mWaitingList.clear(); + mLastVisitedEntry = NULL; } void LLViewerRegion::unpackRegionHandshake() diff --git a/indra/newview/llviewerregion.h b/indra/newview/llviewerregion.h index a2d07d0070..28b541d03e 100644 --- a/indra/newview/llviewerregion.h +++ b/indra/newview/llviewerregion.h @@ -478,6 +478,9 @@ class LLViewerRegion: public LLCapabilityProvider // implements this interface void addCacheMiss(U32 id, LLViewerRegion::eCacheMissType miss_type); void decodeBoundingInfo(LLVOCacheEntry* entry); bool isNonCacheableObjectCreated(U32 local_id); + // Whether the region's object cache is to be removed, letting go of its + // entries, before anything made from them would be. + bool cacheRemovalRequested() const; public: void applyCacheMiscExtras(LLViewerObject* obj); diff --git a/indra/newview/llvocache.cpp b/indra/newview/llvocache.cpp index a7d950cf7e..85830e585f 100644 --- a/indra/newview/llvocache.cpp +++ b/indra/newview/llvocache.cpp @@ -39,6 +39,7 @@ #include +#include #include #include @@ -273,6 +274,31 @@ LLVOCacheEntry::~LLVOCacheEntry() mDP.freeBuffer(); } +// static +void LLVOCacheEntry::releaseAll(vocache_entry_map_t& entries, std::initializer_list sets, + const std::function& leave_tree, std::vector>& keep) +{ + keep.reserve(keep.size() + entries.size()); + for (auto& [local_id, entry] : entries) + { + if (entry.isNull()) + { + continue; + } + leave_tree(entry); + entry->removeOctreeEntry(); + entry->setValid(false); + keep.push_back(std::move(entry)); + } + entries.clear(); + + // Every entry a set can hold is one of the map's. + for (vocache_entry_set_t* set : sets) + { + set->clear(); + } +} + void LLVOCacheEntry::updateEntry(U32 crc, LLDataPackerBinaryBuffer &dp) { if(mCRC != crc) @@ -1426,6 +1452,29 @@ void LLVOCache::removeEntry(U64 handle) removeEntry(entry) ; } +void LLVOCache::requestRemoval(U64 handle) +{ + if (!isRemovalRequested(handle)) + { + mRequestedRemovals.push_back(handle); + } +} + +bool LLVOCache::isRemovalRequested(U64 handle) const +{ + return std::find(mRequestedRemovals.begin(), mRequestedRemovals.end(), handle) != mRequestedRemovals.end(); +} + +void LLVOCache::removeRequestedEntries() +{ + std::vector handles; + handles.swap(mRequestedRemovals); + for (U64 handle : handles) + { + removeEntry(handle); + } +} + void LLVOCache::clearCacheInMemory() { if(!mHeaderEntryQueue.empty()) diff --git a/indra/newview/llvocache.h b/indra/newview/llvocache.h index 4bb2116401..38427e58a6 100644 --- a/indra/newview/llvocache.h +++ b/indra/newview/llvocache.h @@ -36,6 +36,10 @@ #include #include +#include +#include +#include + //--------------------------------------------------------------------------- // Cache entries class LLCamera; @@ -210,6 +214,12 @@ class LLVOCacheEntry typedef boost::unordered_map vocache_gltf_overrides_map_t; + // Lets every entry of a region's map go, leaving nothing that points at one: each leaves the cache's tree + // through `leave_tree` and the octree entry it may share with a drawable, and is marked invalid, and `sets`, + // which hold bare pointers to the map's entries, are emptied. The map's references go to `keep`. + static void releaseAll(vocache_entry_map_t& entries, std::initializer_list sets, + const std::function& leave_tree, std::vector>& keep); + // [SL:KB] - Patch: World-Derender | Checked: 2014-08-10 (Catznip-3.7) vocache_entry_set_t::const_iterator getChildrenBegin() const { return mChildrenList.begin(); } vocache_entry_set_t::const_iterator getChildrenEnd() const { return mChildrenList.end(); } @@ -347,6 +357,11 @@ class LLVOCache : public LLParamSingleton void writeToCache(U64 handle, const LLUUID& id, const LLVOCacheEntry::vocache_entry_map_t& cache_entry_map, bool dirty_cache, bool removal_enabled); void writeGenericExtrasToCache(U64 handle, const LLUUID& id, const LLVOCacheEntry::vocache_gltf_overrides_map_t& cache_extras_entry_map, bool dirty_cache, bool removal_enabled); void removeEntry(U64 handle) ; + // Removes a region's entry as removeEntry does, once removeRequestedEntries runs: the removal lets go of the + // region's objects in memory, one of which an object update asking for it is read from. + void requestRemoval(U64 handle); + bool isRemovalRequested(U64 handle) const; + void removeRequestedEntries(); void removeGenericExtrasForHandle(U64 handle); U32 getCacheEntries() { return mNumEntries; } @@ -378,6 +393,7 @@ class LLVOCache : public LLParamSingleton std::filesystem::path mObjectCacheDirPath; header_entry_queue_t mHeaderEntryQueue; handle_entry_map_t mHandleEntryMap; + std::vector mRequestedRemovals; }; #endif diff --git a/indra/newview/llvovolume.cpp b/indra/newview/llvovolume.cpp index 5ac98499dc..e1e90e208b 100644 --- a/indra/newview/llvovolume.cpp +++ b/indra/newview/llvovolume.cpp @@ -451,13 +451,16 @@ U32 LLVOVolume::processUpdateMessage(LLMessageSystem *mesgsys, if (cut != ALCompressedVolume::ECut::TEXTURE_ANIM) { // An update that ends before the end of its texture - // entry is bogus data, which flushes the region's cache. + // entry is bogus data, which flushes the region's cache + // once the update is done with: the flush lets go of the + // region's cached objects, and the update may be read + // from one of them. dp->dumpBufferToLog(); LL_WARNS() << "Flushing cache files" << LL_ENDL; if(LLVOCache::instanceExists() && getRegion()) { - LLVOCache::getInstance()->removeEntry(getRegion()->getHandle()) ; + LLVOCache::getInstance()->requestRemoval(getRegion()->getHandle()); } } LL_WARNS("UpdateFail") << (cut == ALCompressedVolume::ECut::VOLUME_PARAMS ? "Volume parameters" diff --git a/indra/newview/llworld.cpp b/indra/newview/llworld.cpp index 1c87c7c0ff..7a6c5d7bf8 100644 --- a/indra/newview/llworld.cpp +++ b/indra/newview/llworld.cpp @@ -695,6 +695,13 @@ void LLWorld::updateRegions(F32 max_update_time) // Before the regions' updates, since it may remove one. mRegionRetirement.update(); + // The cache removals object updates asked for, which let go of regions' + // cached objects, before the regions' updates walk them. + if (LLVOCache::instanceExists()) + { + LLVOCache::getInstance()->removeRequestedEntries(); + } + LLViewerRegion::calcNewObjectCreationThrottle(); if(LLViewerRegion::isNewObjectCreationThrottleDisabled()) { diff --git a/indra/newview/tests/llvieweroctree_stub.cpp b/indra/newview/tests/llvieweroctree_stub.cpp index e850efd9ac..36193a5d35 100644 --- a/indra/newview/tests/llvieweroctree_stub.cpp +++ b/indra/newview/tests/llvieweroctree_stub.cpp @@ -75,6 +75,9 @@ void LLViewerOctreeEntryData::setSpatialExtents(const LLVector4a& min, const LLV void LLViewerOctreeEntryData::setPositionGroup(const LLVector4a& pos) {} void LLViewerOctreeEntryData::setGroup(LLViewerOctreeGroup* group) {} void LLViewerOctreeEntryData::setOctreeEntry(LLViewerOctreeEntry* entry) {} +// The entries each removeOctreeEntry was called on, in order. +std::vector sRemovedOctreeEntries; +void LLViewerOctreeEntryData::removeOctreeEntry() { sRemovedOctreeEntries.push_back(this); } U32 LLViewerOctreeEntryData::sCurVisible{}; LLViewerOctreePartition::LLViewerOctreePartition() = default; diff --git a/indra/newview/tests/llvocache_test.cpp b/indra/newview/tests/llvocache_test.cpp index 1673b3fa32..c6ffff6b38 100644 --- a/indra/newview/tests/llvocache_test.cpp +++ b/indra/newview/tests/llvocache_test.cpp @@ -42,6 +42,7 @@ #include "llvieweroctree_stub.cpp" +#include #include #include @@ -807,6 +808,66 @@ namespace CHECK_FALSE(std::filesystem::exists(objectFile(handle, "_extras.slec"))); } + // Removing a region's entry lets go of its objects in memory, and an + // object update that finds the cache bogus is read from one of them. + TEST_CASE_METHOD(vocacheTest, "LLVOCache: a removal asked for waits for the removals asked for to be carried out", "[LLVOCache]") + { + const U64 handle = to_region_handle(1023 * 256, 1000 * 256); + const LLUUID id = LLUUID::generateNewID(); + LLVOCache::instance().writeToCache(handle, id, makeEntries(3), true, false); + REQUIRE(LLVOCache::instance().getCacheEntries() == 1u); + + CHECK_FALSE(LLVOCache::instance().isRemovalRequested(handle)); + LLVOCache::instance().requestRemoval(handle); + LLVOCache::instance().requestRemoval(handle); + UNSCOPED_INFO("the region is still cached once its removal is asked for"); + CHECK(LLVOCache::instance().isRemovalRequested(handle)); + CHECK(LLVOCache::instance().getCacheEntries() == 1u); + CHECK(std::filesystem::exists(objectFile(handle, ".slc"))); + + LLVOCache::instance().removeRequestedEntries(); + UNSCOPED_INFO("and gone once the removals are carried out"); + CHECK_FALSE(LLVOCache::instance().isRemovalRequested(handle)); + CHECK(LLVOCache::instance().getCacheEntries() == 0u); + CHECK_FALSE(std::filesystem::exists(objectFile(handle, ".slc"))); + + UNSCOPED_INFO("which leaves none asked for"); + LLVOCache::instance().writeToCache(handle, id, makeEntries(3), true, false); + LLVOCache::instance().removeRequestedEntries(); + CHECK(LLVOCache::instance().getCacheEntries() == 1u); + } + + TEST_CASE("LLVOCacheEntry: releaseAll leaves no set holding an entry it let go", "[LLVOCache]") + { + LLVOCacheEntry::vocache_entry_map_t entries = vocacheTest::makeEntries(6); + const LLPointer watched = entries[2]; + LLVOCacheEntry::vocache_entry_set_t active; + LLVOCacheEntry::vocache_entry_set_t visible; + active.insert(entries[1].get()); + active.insert(watched.get()); + visible.insert(entries[5].get()); + + std::vector left_tree; + std::vector> keep; + sRemovedOctreeEntries.clear(); + LLVOCacheEntry::releaseAll(entries, { &active, &visible }, [&](LLVOCacheEntry* entry) { left_tree.push_back(entry); }, keep); + + CHECK(entries.empty()); + CHECK(active.empty()); + CHECK(visible.empty()); + UNSCOPED_INFO("every entry left the tree"); + CHECK(left_tree.size() == 6u); + CHECK(std::find(left_tree.begin(), left_tree.end(), watched.get()) != left_tree.end()); + UNSCOPED_INFO("and its octree entry"); + CHECK(sRemovedOctreeEntries.size() == 6u); + const LLViewerOctreeEntryData* watched_data = watched.get(); + CHECK(std::find(sRemovedOctreeEntries.begin(), sRemovedOctreeEntries.end(), watched_data) != sRemovedOctreeEntries.end()); + UNSCOPED_INFO("and is held by keep alone besides the test"); + CHECK(keep.size() == 6u); + CHECK(watched->getNumRefs() == 2); + CHECK_FALSE(watched->isValid()); + } + TEST_CASE_METHOD(vocacheTest, "LLVOCache: staysInMemory keeps entries seen lately or inside the rear sphere", "[LLVOCache]") { // A stand-in for an entry that nothing has looked at in a while: its