From 17cb89208c804680ffd4445d6a826171a67edb79 Mon Sep 17 00:00:00 2001
From: Dmitry Konstantinov <netudima@gmail.com>
Date: Fri, 7 Mar 2025 23:24:22 +0300
Subject: [PATCH] Fix marking an SSTable as suspected and BufferPool leakage in
 case of a corrupted SSTable read during a compaction

Patch by Dmitry Konstantinov; reviewed by Branimir Lambov for CASSANDRA-20396
---
 .../apache/cassandra/cache/ChunkCache.java    | 12 +++++-
 .../io/sstable/SSTableIdentityIterator.java   | 30 +++++++++++++
 .../sstable/format/SSTableSimpleScanner.java  | 42 ++++++++++++++-----
 3 files changed, 72 insertions(+), 12 deletions(-)

diff --git a/src/java/org/apache/cassandra/cache/ChunkCache.java b/src/java/org/apache/cassandra/cache/ChunkCache.java
index e7e50296ac..5fb0bc6a2f 100644
--- a/src/java/org/apache/cassandra/cache/ChunkCache.java
+++ b/src/java/org/apache/cassandra/cache/ChunkCache.java
@@ -162,8 +162,16 @@ public class ChunkCache
     {
         ByteBuffer buffer = bufferPool.get(key.file.chunkSize(), key.file.preferredBufferType());
         assert buffer != null;
-        key.file.readChunk(key.position, buffer);
-        return new Buffer(buffer, key.position);
+        try
+        {
+            key.file.readChunk(key.position, buffer);
+            return new Buffer(buffer, key.position);
+        }
+        catch (Throwable t)
+        {
+            bufferPool.put(buffer);
+            throw t;
+        }
     }
 
     @Override
diff --git a/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java b/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java
index 072a364af3..d5a1ae8bcc 100644
--- a/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java
+++ b/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java
@@ -76,6 +76,11 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat
             sstable.markSuspect();
             throw new CorruptSSTableException(e, file.getPath());
         }
+        catch (CorruptSSTableException e) // to ensure that we marked the sstable as suspected if CorruptSSTableException is thrown from lower levels
+        {
+            sstable.markSuspect();
+            throw e;
+        }
     }
 
     public static SSTableIdentityIterator create(SSTableReader sstable, FileDataInput dfile, long dataPosition, DecoratedKey key, boolean tombstoneOnly)
@@ -99,6 +104,11 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat
             sstable.markSuspect();
             throw new CorruptSSTableException(e, dfile.getPath());
         }
+        catch (CorruptSSTableException e) // to ensure that we marked the sstable as suspected if CorruptSSTableException is thrown from lower levels
+        {
+            sstable.markSuspect();
+            throw e;
+        }
     }
 
     public static SSTableIdentityIterator create(SSTableReader sstable, FileDataInput dfile, boolean tombstoneOnly)
@@ -121,6 +131,11 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat
             sstable.markSuspect();
             throw new CorruptSSTableException(e, dfile.getPath());
         }
+        catch (CorruptSSTableException e) // to ensure that we marked the sstable as suspected if CorruptSSTableException is thrown from lower levels
+        {
+            sstable.markSuspect();
+            throw e;
+        }
     }
 
     public TableMetadata metadata()
@@ -164,6 +179,11 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat
             sstable.markSuspect();
             throw new CorruptSSTableException(e, filename);
         }
+        catch (CorruptSSTableException e) // to ensure that we marked the sstable as suspected if CorruptSSTableException is thrown from lower levels
+        {
+            sstable.markSuspect();
+            throw e;
+        }
         catch (IOError e)
         {
             if (e.getCause() instanceof IOException)
@@ -192,6 +212,11 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat
             sstable.markSuspect();
             throw new CorruptSSTableException(e, filename);
         }
+        catch (CorruptSSTableException e) // to ensure that we marked the sstable as suspected if CorruptSSTableException is thrown from lower levels
+        {
+            sstable.markSuspect();
+            throw e;
+        }
         catch (IOError e)
         {
             if (e.getCause() instanceof IOException)
@@ -240,6 +265,11 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat
             sstable.markSuspect();
             throw new CorruptSSTableException(e, filename);
         }
+        catch (CorruptSSTableException e) // to ensure that we marked the sstable as suspected if CorruptSSTableException is thrown from lower levels
+        {
+            sstable.markSuspect();
+            throw e;
+        }
         catch (IOError e)
         {
             if (e.getCause() instanceof IOException)
diff --git a/src/java/org/apache/cassandra/io/sstable/format/SSTableSimpleScanner.java b/src/java/org/apache/cassandra/io/sstable/format/SSTableSimpleScanner.java
index 1789aff72c..a649fbea4c 100644
--- a/src/java/org/apache/cassandra/io/sstable/format/SSTableSimpleScanner.java
+++ b/src/java/org/apache/cassandra/io/sstable/format/SSTableSimpleScanner.java
@@ -17,6 +17,8 @@
  */
 package org.apache.cassandra.io.sstable.format;
 
+import java.io.IOError;
+import java.io.IOException;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
@@ -148,19 +150,39 @@ implements ISSTableScanner
 
     boolean advanceRange()
     {
-        if (!rangeIterator.hasNext())
-            return false;
+        try
+        {
+            if (!rangeIterator.hasNext())
+                return false;
 
-        bytesScannedInPreviousRanges += currentEndPosition - currentStartPosition;
+            bytesScannedInPreviousRanges += currentEndPosition - currentStartPosition;
 
-        PartitionPositionBounds nextRange = rangeIterator.next();
-        if (currentEndPosition > nextRange.lowerPosition)
-            throw new IllegalArgumentException("Ranges supplied to SSTableSimpleScanner must be non-overlapping and in ascending order.");
+            PartitionPositionBounds nextRange = rangeIterator.next();
+            if (currentEndPosition > nextRange.lowerPosition)
+                throw new IllegalArgumentException("Ranges supplied to SSTableSimpleScanner must be non-overlapping and in ascending order.");
 
-        currentEndPosition = nextRange.upperPosition;
-        currentStartPosition = nextRange.lowerPosition;
-        dfile.seek(currentStartPosition);
-        return true;
+            currentEndPosition = nextRange.upperPosition;
+            currentStartPosition = nextRange.lowerPosition;
+            dfile.seek(currentStartPosition);
+            return true;
+        }
+        catch (CorruptSSTableException e)
+        {
+            sstable.markSuspect();
+            throw e;
+        }
+        catch (IOError e)
+        {
+            if (e.getCause() instanceof IOException)
+            {
+                sstable.markSuspect();
+                throw new CorruptSSTableException((Exception)e.getCause(), sstable.getFilename());
+            }
+            else
+            {
+                throw e;
+            }
+        }
     }
 
     public UnfilteredRowIterator next()