From 45735addba3d67d8dbf6e243dd9f16ffa8199e2e Mon Sep 17 00:00:00 2001
From: Gavin Halliday <gavin.halliday@lexisnexis.com>
Date: Tue, 2 Apr 2024 09:49:57 +0100
Subject: [PATCH] Update comment and improve error reporting

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
---
 system/jhtree/jhtree.cpp | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/system/jhtree/jhtree.cpp b/system/jhtree/jhtree.cpp
index b74ea9d286e..e98840bda02 100644
--- a/system/jhtree/jhtree.cpp
+++ b/system/jhtree/jhtree.cpp
@@ -690,7 +690,7 @@ class CNodeMapping : public HTMapping<CNodeCacheEntry, CKeyIdAndPos>
 };
 
 typedef OwningSimpleHashTableOf<CNodeMapping, CKeyIdAndPos> CNodeTable;
-class CNodeMRUCache : public CMRUCacheOf<CKeyIdAndPos, CNodeCacheEntry, CNodeMapping, CNodeTable>
+class CNodeMRUCache final : public CMRUCacheOf<CKeyIdAndPos, CNodeCacheEntry, CNodeMapping, CNodeTable>
 {
     std::atomic<size32_t> sizeInMem{0};
     size32_t memLimit = 0;
@@ -706,10 +706,13 @@ class CNodeMRUCache : public CMRUCacheOf<CKeyIdAndPos, CNodeCacheEntry, CNodeMap
     virtual void makeSpace()
     {
         // remove LRU until !full
+        // This code could walk the list, rather than restarting at the end each time - but there are unlikely to be
+        // many entries that have no associated node, and nodes could have been associated in the meantime.
         do
         {
             CNodeMapping *tail = mruList.tail();
-            assertex(tail);
+            if (unlikely(!tail))
+                throw makeStringExceptionV(9999, "Index cache appears full but contains no entries size=%x limit=%x", sizeInMem, memLimit);
 
             //Never evict an entry that hasn't yet loaded - otherwise the sizeInMem can become inconsistent
             //When running with slow remote storage this can take a long time to be ready - so we need
@@ -719,7 +722,11 @@ class CNodeMRUCache : public CMRUCacheOf<CKeyIdAndPos, CNodeCacheEntry, CNodeMap
             {
                 tail = tail->prev;
                 if (!tail)
-                    return; // no pages in the cache are ready - this should never happen because full() should be false
+                {
+                     // no pages in the cache are ready - this could possibly happen in a tiny race-window where
+                     // sizes in the cache have been updated, but no nodes have yet been associated with the entries.
+                    return;
+                }
             }
 
             mruList.remove(tail);