Protect uncaught_exceptions virtual table against the crash path

src/java/org/apache/cassandra/db/virtual/ExceptionsTable.java

@@ -52,6 +52,14 @@ public class ExceptionsTable extends AbstractMutableVirtualTable
      */
     static final List<ExceptionRow> preInitialisationBuffer = Collections.synchronizedList(new ArrayList<>());
 
+    /**
+     * Upper bound on {@link #preInitialisationBuffer}. Bounds heap retention if uncaught exceptions storm during early
+     * startup (before virtual tables are registered), or in offline/tool contexts that never register virtual tables
+     * and thus never call {@link #flush()}. Kept in line with the live buffer's default cap.
+     */
+    @VisibleForTesting
+    static final int PRE_INITIALISATION_BUFFER_CAPACITY = 1000;
+
     @VisibleForTesting
     static volatile ExceptionsTable INSTANCE;
 
@@ -62,7 +70,7 @@ public class ExceptionsTable extends AbstractMutableVirtualTable
     ExceptionsTable(String keyspace)
     {
         // for starters capped to 1k, I do not think we need to make this configurable (yet).
-        this(keyspace, 1000);
+        this(keyspace, PRE_INITIALISATION_BUFFER_CAPACITY);
     }
 
     ExceptionsTable(String keyspace, int maxSize)
@@ -84,10 +92,19 @@ public class ExceptionsTable extends AbstractMutableVirtualTable
 
     public void flush()
     {
-        for (ExceptionRow row : preInitialisationBuffer)
-            add(row.exceptionClass, row.exceptionLocation, row.message, row.stackTrace, row.occurrence.getTime());
+        // Drain under the list's monitor and iterate a private copy: preInitialisationBuffer is a synchronizedList,
+        // whose contract requires holding its monitor while iterating. A concurrent persist() on another thread could
+        // otherwise add() during iteration and trigger a ConcurrentModificationException, which would propagate out of
+        // setupVirtualKeyspaces() and abort node startup.
+        List<ExceptionRow> drained;
+        synchronized (preInitialisationBuffer)
+        {
+            drained = new ArrayList<>(preInitialisationBuffer);
+            preInitialisationBuffer.clear();
+        }
 
-        preInitialisationBuffer.clear();
+        for (ExceptionRow row : drained)
+            add(row.exceptionClass, row.exceptionLocation, row.message, row.stackTrace, row.occurrence.getTime());
     }
 
     @Override
@@ -178,12 +195,19 @@ public static void persist(Throwable t)
         }
         else
         {
-            preInitialisationBuffer.add(new ExceptionRow(toPersist.getClass().getName(),
-                                                         stackTrace.isEmpty() ? "unknown" : stackTrace.get(0),
-                                                         0,
-                                                         toPersist.getMessage(),
-                                                         stackTrace,
-                                                         now));
+            // Bound retention (see PRE_INITIALISATION_BUFFER_CAPACITY): keep the earliest entries, which are usually the
+            // most diagnostic, and drop once full rather than growing without limit. Guard the size check and the add
+            // together under the list monitor so concurrent persist() calls cannot race past the cap.
+            synchronized (preInitialisationBuffer)
+            {
+                if (preInitialisationBuffer.size() < PRE_INITIALISATION_BUFFER_CAPACITY)
+                    preInitialisationBuffer.add(new ExceptionRow(toPersist.getClass().getName(),
+                                                                 stackTrace.isEmpty() ? "unknown" : stackTrace.get(0),
+                                                                 0,
+                                                                 toPersist.getMessage(),
+                                                                 stackTrace,
+                                                                 now));
+            }
         }
     }
 

src/java/org/apache/cassandra/utils/JVMStabilityInspector.java

@@ -74,7 +74,9 @@ public static void uncaughtException(Thread thread, Throwable t)
         try { StorageMetrics.uncaughtExceptions.inc(); } catch (Throwable ignore) { /* might not be initialised */ }
         logger.error("Exception in thread {}", thread, t);
         Tracing.trace("Exception in thread {}", thread, t);
-        ExceptionsTable.persist(t);
+        // Recording the exception for observability must never preempt the stability handling below (the
+        // disk_failure_policy / OOM "die" actions in inspectThrowable). Guard it like the StorageMetrics increment above.
+        try { ExceptionsTable.persist(t); } catch (Throwable ignore) { /* observability only, must not throw here */ }
         for (Throwable t2 = t; t2 != null; t2 = t2.getCause())
         {
             // make sure error gets logged exactly once.

test/unit/org/apache/cassandra/db/virtual/ExceptionsTableTest.java

@@ -287,6 +287,25 @@ public void testEmptyStacktrace()
         });
     }
 
+    @Test
+    public void testPreInitialisationBufferIsBounded()
+    {
+        doWithVTable(100, table ->
+        {
+            // Do not register the table, so INSTANCE stays null and every persist() lands in the pre-initialisation
+            // buffer. Persisting well past the cap must not grow the buffer without bound.
+            ExceptionsTable.INSTANCE = null;
+            ExceptionsTable.preInitialisationBuffer.clear();
+
+            int overCap = ExceptionsTable.PRE_INITIALISATION_BUFFER_CAPACITY + 50;
+            for (int i = 0; i < overCap; i++)
+                ExceptionsTable.persist(new MyUncaughtException("boom " + i));
+
+            assertEquals(ExceptionsTable.PRE_INITIALISATION_BUFFER_CAPACITY,
+                         ExceptionsTable.preInitialisationBuffer.size());
+        });
+    }
+
     private List<UntypedResultSet.Row> rows(String query)
     {
         return execute(query).stream().collect(toList());