fix(qwp): fold recovery CRC over Unsafe; broaden the mmap-fault match

Recovery reads a mapped .sfa directly; an unbacked page (a ZFS sparse
pre-allocation tail, or a torn write leaving a real payloadLen pointing
into an unwritten hole) faults on read. Two gaps on the shipping JDK 8:

- The mmap-fault guard matched "a fault occurred in an unsafe memory
  access", which is only the JDK 21+ wording. Pre-21 (incl. JDK 8) emits
  "...a recent unsafe memory access operation in compiled Java code", so
  the guard was inert there. Match the shared fragment
  "unsafe memory access operation".

- The payload CRC used the native Crc32c over the mapping. A SIGBUS
  inside JNI is not translated into a catchable InternalError (that
  happens only at Unsafe intrinsic sites), so a payload reaching an
  unbacked page aborted the whole JVM before the CRC check could reject
  the frame. Fold the recovery-time CRC over Unsafe reads (table-based
  CRC-32C, bit-identical to native), so the fault is catchable or
  degrades to a CRC mismatch. Native Crc32c stays on the append hot path.
  (A pre-touch guard before the native read was tried and proven
  unreliable: once an earlier native CRC ran in the same scan, it does
  not reliably fault first.)

Rewrite the tests to drive the production openExisting path rather than
reflecting into scanFrames: on pre-21 the imprecise fault escapes a
reflective invoke frame, so reflection-based tests spuriously failed on
JDK 8/11/17 even though the direct-call production path catches it.
Induce the unbacked tail portably (truncate down then back up to leave a
sparse hole) and size off Files.PAGE_SIZE.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: puzpuzpuz

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/MmapSegment.java

@@ -64,6 +64,7 @@ public final class MmapSegment implements QuietCloseable {
     public static final int FRAME_HEADER_SIZE = 8;   // u32 crc + u32 payloadLen
     public static final int HEADER_SIZE = 24;
     public static final byte VERSION = 1;
+    private static final int[] CRC32C_TABLE = buildCrc32cTable();
     private static final Logger LOG = LoggerFactory.getLogger(MmapSegment.class);
 
     private final String path;
@@ -509,22 +510,30 @@ public long tornTailBytes() {
     /**
      * True when {@code t} is the JVM's recoverable signal for a fault while
      * accessing a memory-mapped region -- a SIGBUS/SIGSEGV that HotSpot
-     * translates into {@code InternalError("a fault occurred in an unsafe
-     * memory access operation")} instead of aborting the process. It surfaces
-     * when a mapped page is not backed by real file blocks: a sparse
-     * {@code .sfa} tail on a filesystem whose pre-allocation leaves holes
-     * (e.g. ZFS, where a truncate-based {@code allocate} does not materialize
-     * blocks), or a file truncated under the mapping. Recovery treats this as
-     * an I/O boundary -- the same way MappedByteBuffer readers do -- not a
-     * fatal VM error. Matches on the JVM's stable message so a genuine
-     * VirtualMachineError (real OOM, StackOverflow) is never swallowed.
+     * translates into an {@code InternalError} at an {@code Unsafe} intrinsic
+     * site instead of aborting the process. It surfaces when a mapped page is
+     * not backed by real file blocks: a sparse {@code .sfa} tail on a
+     * filesystem whose pre-allocation leaves holes (e.g. ZFS, where a
+     * truncate-based {@code allocate} does not materialize blocks), or a file
+     * truncated under the mapping. Recovery treats this as an I/O boundary --
+     * the same way MappedByteBuffer readers do -- not a fatal VM error.
+     * <p>
+     * The message is matched on the fragment {@code "unsafe memory access
+     * operation"}, which is common to both HotSpot wordings and NOT
+     * version-stable as a whole: pre-21 JDKs (including the shipping/CI JDK 8)
+     * emit {@code "a fault occurred in a recent unsafe memory access operation
+     * in compiled Java code"}, while JDK 21+ shortened it to {@code "a fault
+     * occurred in an unsafe memory access operation"}. Matching the shared
+     * fragment keeps the guard effective on JDK 8/11/17 as well as 21+, while
+     * still being specific enough that a genuine VirtualMachineError (real OOM,
+     * StackOverflow) is never swallowed.
      */
     private static boolean isMmapAccessFault(Throwable t) {
         if (!(t instanceof InternalError)) {
             return false;
         }
         String msg = t.getMessage();
-        return msg != null && msg.contains("a fault occurred in an unsafe memory access");
+        return msg != null && msg.contains("unsafe memory access operation");
     }
 
     /**
@@ -545,10 +554,26 @@ private static long scanFrames(long addr, long fileSize) {
                 if (payloadLen < 0 || pos + FRAME_HEADER_SIZE + payloadLen > fileSize) {
                     return pos;
                 }
-                int crcCalc = Crc32c.update(Crc32c.INIT, addr + pos + 4, 4);
-                if (payloadLen > 0) {
-                    crcCalc = Crc32c.update(crcCalc, addr + pos + FRAME_HEADER_SIZE, payloadLen);
-                }
+                // CRC over the contiguous (payloadLen, payload) pair, folded
+                // via Unsafe reads rather than the native Crc32c.update.
+                // Recovery maps to the file's stat length, but a page inside
+                // that range can be unbacked: a sparse pre-allocation tail (a
+                // truncate-based allocate that never materialized blocks, as on
+                // ZFS), or -- via a torn write, since tryAppend writes the
+                // length field before copying the payload -- a real positive
+                // payloadLen whose payload spans into an unwritten hole. A raw
+                // read of an unbacked page raises SIGBUS; HotSpot translates
+                // that into a catchable InternalError ONLY at an Unsafe
+                // intrinsic site, NEVER inside JNI native code, so a native
+                // Crc32c.update over such a page aborts the whole JVM (and,
+                // empirically on pre-21 JDKs, a preceding Unsafe pre-touch does
+                // not reliably fault first once an earlier native CRC ran in
+                // the same scan). Folding over Unsafe keeps every fault
+                // catchable -- handled below as the boundary of recoverable
+                // data; a page that instead reads back as zeroes just fails the
+                // CRC check and ends the scan. Recovery is cold, so the slower
+                // table CRC here is immaterial.
+                int crcCalc = crc32cRecovery(addr + pos + 4, 4L + payloadLen);
                 if (crcCalc != crcRead) {
                     return pos;
                 }
@@ -577,6 +602,39 @@ private static long scanFrames(long addr, long fileSize) {
         return pos;
     }
 
+    /**
+     * CRC-32C (Castagnoli) of {@code [addr, addr + len)} read through
+     * {@link Unsafe}, seeded like {@code Crc32c.update(Crc32c.INIT, addr, len)}
+     * and bit-identical to it (verified) -- but every byte load is an Unsafe
+     * intrinsic, so a fault on an unbacked mapped page is a catchable
+     * {@link InternalError} instead of the uncatchable JNI SIGBUS the native
+     * {@link Crc32c} would raise. Byte-at-a-time via a precomputed table
+     * ({@code ~0.5 GiB/s}); used only on the cold recovery scan, never on the
+     * append hot path (which stays on the native, hardware-friendly path).
+     */
+    private static int crc32cRecovery(long addr, long len) {
+        int crc = ~Crc32c.INIT;
+        for (long i = 0; i < len; i++) {
+            crc = (crc >>> 8) ^ CRC32C_TABLE[(crc ^ Unsafe.getUnsafe().getByte(addr + i)) & 0xFF];
+        }
+        return ~crc;
+    }
+
+    // Standard reflected CRC-32C byte table (polynomial 0x82F63B78), matching
+    // crc32c_table[0] in the native crc32c.c. Computed at class init to avoid
+    // 256 hand-transcribed literals; drives crc32cRecovery.
+    private static int[] buildCrc32cTable() {
+        int[] table = new int[256];
+        for (int n = 0; n < 256; n++) {
+            int c = n;
+            for (int k = 0; k < 8; k++) {
+                c = (c & 1) != 0 ? (0x82F63B78 ^ (c >>> 1)) : (c >>> 1);
+            }
+            table[n] = c;
+        }
+        return table;
+    }
+
     /**
      * Distinguishes "torn tail" (writer attempted a write past the last valid
      * frame and failed — partial write, mid-stream corruption, bit rot) from

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/MmapSegmentRecoveryFaultTest.java

@@ -33,7 +33,6 @@
 import org.junit.Before;
 import org.junit.Test;
 
-import java.lang.reflect.Method;
 import java.nio.file.Paths;
 
 import static org.junit.Assert.assertEquals;
@@ -42,25 +41,35 @@
 /**
  * Regression guard for the recovery-time SIGBUS hazard in {@link MmapSegment}.
  * <p>
- * {@code openExisting} maps a recovered {@code .sfa} to its stat length and
- * {@code scanFrames} / {@code detectTornTail} read the mapping directly. When a
- * prior session left a sparse segment tail -- a truncate-based pre-allocation
- * that never materialized the tail blocks, as happens on ZFS -- a read of an
+ * On recovery, {@link MmapSegment#openExisting} maps a persisted {@code .sfa}
+ * to its stat length and scans frames straight out of the mapping. When a prior
+ * session left a sparse segment tail -- a truncate-based pre-allocation that
+ * never materialized the tail blocks, as happens on ZFS -- a read of an
  * unbacked page raises the JVM's recoverable
- * {@code InternalError("a fault occurred in an unsafe memory access
- * operation")} (a translated SIGBUS). That error is NOT a
- * {@code MmapSegmentException}, so {@code SegmentRing.openExisting}'s per-file
- * skip did not catch it: it aborted recovery of the whole slot, which surfaced
- * (via a drainer/probe) as a spurious "unsafe memory access" failure on ZFS
- * CI runners.
+ * {@code InternalError("...unsafe memory access operation...")} (a translated
+ * SIGBUS). Recovery must treat that page as the boundary of recoverable data,
+ * keep every frame below it, and hand back a usable segment -- not let the
+ * error abort recovery of the whole slot (the reported ZFS-CI flake).
  * <p>
- * The fault only reproduces on a real filesystem whose mmap reads of unwritten
- * regions fault instead of zero-filling (ZFS), so this test induces the very
- * same JVM-recoverable fault deterministically on any filesystem: it maps a
- * valid segment file, then truncates the backing file under the still-larger
- * mapping so the tail page is genuinely beyond EOF (the one case POSIX mmap
- * always faults on). The scan must then stop at that page and keep every frame
- * below it, exactly as it treats a torn tail.
+ * These tests drive the <b>production entry point</b> ({@code openExisting}),
+ * not the private scan methods via reflection. That matters for two reasons:
+ * <ul>
+ *   <li>It exercises the real recovery path end to end.</li>
+ *   <li>On pre-21 JDKs the mmap-fault {@code InternalError} is delivered
+ *       imprecisely ("a fault occurred in <i>a recent</i> unsafe memory access
+ *       operation in compiled Java code") and escapes a <i>reflective</i>
+ *       {@code Method.invoke} frame instead of being caught inside the scan --
+ *       so a reflection-based test spuriously fails on the shipping JDK 8/11/17
+ *       even though the direct-call production path catches it fine.</li>
+ * </ul>
+ * The unbacked tail is produced portably by truncating the file down (dropping
+ * the tail blocks) and back up to the mapping size (leaving a sparse hole). A
+ * hole-faulting filesystem (ZFS) then faults on the read exactly as in
+ * production -- the case the fix must survive rather than fold the CRC through
+ * the native, JNI-side {@code Crc32c} where a SIGBUS is uncatchable and aborts
+ * the JVM. A hole-zero-filling filesystem (ext4) instead reads the hole back as
+ * zeroes, which fails the frame CRC; either way recovery must stop at the same
+ * boundary and recover the same frames.
  */
 public class MmapSegmentRecoveryFaultTest {
 
@@ -98,61 +107,117 @@ public void tearDown() {
         Files.remove(tmpDir);
     }
 
+    /**
+     * Clean unbacked tail: a single frame ends exactly on a page boundary and
+     * everything above it is a sparse hole. Recovery must keep the frame and
+     * stop at the boundary, reporting no torn tail (an unwritten hole is not a
+     * torn write).
+     */
     @Test
-    public void testRecoveryScanTreatsUnbackedTailAsBoundary() throws Exception {
+    public void testRecoveryKeepsFramesBeforeUnbackedTail() throws Exception {
         TestUtils.assertMemoryLeak(() -> {
             final String path = tmpDir + "/seg-unbacked-tail.sfa";
-            // One frame sized so the segment's used region ends exactly on a
-            // 4 KiB page boundary: HEADER_SIZE + FRAME_HEADER_SIZE + payload.
-            // Truncating the backing to that boundary then leaves the NEXT
-            // page entirely beyond EOF -- the deterministic mmap-fault case.
-            final long pageBoundary = 4096L;
-            final int payloadLen = (int) (pageBoundary - MmapSegment.HEADER_SIZE - MmapSegment.FRAME_HEADER_SIZE);
-            long boundary;
-            long buf = Unsafe.malloc(payloadLen, MemoryTag.NATIVE_DEFAULT);
-            try {
-                for (int i = 0; i < payloadLen; i++) {
-                    Unsafe.getUnsafe().putByte(buf + i, (byte) (i | 1)); // all non-zero
-                }
-                try (MmapSegment seg = MmapSegment.create(path, 7L, SEGMENT_BYTES)) {
-                    assertEquals(MmapSegment.HEADER_SIZE, seg.tryAppend(buf, payloadLen));
-                    boundary = seg.publishedOffset();
-                }
-            } finally {
-                Unsafe.free(buf, payloadLen, MemoryTag.NATIVE_DEFAULT);
+            final long page = Files.PAGE_SIZE;
+            // One frame sized so the used region ends exactly on a page
+            // boundary: HEADER_SIZE + FRAME_HEADER_SIZE + payload == page.
+            final int payloadLen = (int) (page - MmapSegment.HEADER_SIZE - MmapSegment.FRAME_HEADER_SIZE);
+
+            long boundary = writeSegment(path, 7L, new int[]{payloadLen});
+            assertEquals("frame must fill exactly one page", page, boundary);
+            // Drop the tail blocks, then re-extend logically so [page, SEGMENT_BYTES)
+            // is an unbacked hole under the recovery mapping.
+            punchSparseTail(path, page);
+
+            try (MmapSegment seg = MmapSegment.openExisting(path)) {
+                assertEquals("the frame below the unbacked tail must be recovered", 1L, seg.frameCount());
+                assertEquals("scan must stop at the unbacked-page boundary", page, seg.publishedOffset());
+                assertEquals("an unwritten hole is not a torn write", 0L, seg.tornTailBytes());
             }
-            assertEquals("frame must fill exactly one page", pageBoundary, boundary);
-
-            // Map the whole segment, then shrink the backing file under the
-            // mapping so [boundary, SEGMENT_BYTES) is unbacked. Reads within
-            // [0, boundary) stay valid; a read at/after `boundary` faults with
-            // the same recoverable InternalError a sparse ZFS tail produces.
-            int fd = Files.openRW(path);
-            assertTrue("openRW failed", fd >= 0);
-            long addr = Files.mmap(fd, SEGMENT_BYTES, 0, Files.MAP_RW, MemoryTag.MMAP_DEFAULT);
-            assertTrue("mmap failed", addr != Files.FAILED_MMAP_ADDRESS);
-            try {
-                assertTrue("truncate failed", Files.truncate(fd, boundary));
-
-                // scanFrames must keep the frame below the unbacked page and
-                // return the boundary rather than propagating the fault.
-                Method scanFrames = MmapSegment.class.getDeclaredMethod(
-                        "scanFrames", long.class, long.class);
-                scanFrames.setAccessible(true);
-                long lastGood = (Long) scanFrames.invoke(null, addr, SEGMENT_BYTES);
-                assertEquals("scan must stop at the unbacked-page boundary", boundary, lastGood);
-
-                // detectTornTail probes the bail-out region -- itself unbacked
-                // here -- and must report clean (0), not a fatal fault.
-                Method detectTornTail = MmapSegment.class.getDeclaredMethod(
-                        "detectTornTail", long.class, long.class, long.class);
-                detectTornTail.setAccessible(true);
-                long torn = (Long) detectTornTail.invoke(null, addr, lastGood, SEGMENT_BYTES);
-                assertEquals("unbacked tail is unwritten space, not a torn write", 0L, torn);
-            } finally {
-                Files.munmap(addr, SEGMENT_BYTES, MemoryTag.MMAP_DEFAULT);
-                Files.close(fd);
+        });
+    }
+
+    /**
+     * The harder case: a frame whose 8-byte header sits on a backed page but
+     * whose payload reaches into the unbacked hole (a torn write leaves a real
+     * positive {@code payloadLen} with the payload spanning the boundary). The
+     * CRC fold therefore reads across the backed-to-unbacked edge. Recovery
+     * must reject that frame and keep the one below it -- and, crucially, must
+     * do so via {@code Unsafe} reads: the native, JNI-side {@code Crc32c} over
+     * an unbacked page raises a SIGBUS that HotSpot cannot translate, aborting
+     * the whole JVM (verified: an {@code hs_err} in
+     * {@code Java_io_questdb_client_std_Crc32c_update}).
+     */
+    @Test
+    public void testRecoverySurvivesPayloadReachingUnbackedPage() throws Exception {
+        TestUtils.assertMemoryLeak(() -> {
+            final String path = tmpDir + "/seg-unbacked-payload.sfa";
+            final long page = Files.PAGE_SIZE;
+            final long boundary = 2 * page;
+            // Frame 2's header ends 8 bytes below the boundary (backed); its
+            // payload starts 8 bytes below and runs a full page past -- across
+            // the backed->unbacked edge.
+            final long frame2Offset = boundary - 16;
+            final int payloadLen2 = (int) page;
+            final int payloadLen1 = (int) (frame2Offset - MmapSegment.HEADER_SIZE - MmapSegment.FRAME_HEADER_SIZE);
+
+            long used = writeSegment(path, 11L, new int[]{payloadLen1, payloadLen2});
+            assertEquals("frame 2's header must end on the page boundary", boundary - 8, frame2Offset + MmapSegment.FRAME_HEADER_SIZE);
+            assertTrue("frame 2 payload must reach past the boundary", used > boundary);
+            punchSparseTail(path, boundary);
+
+            try (MmapSegment seg = MmapSegment.openExisting(path)) {
+                assertEquals("only the frame below the unbacked payload is recoverable", 1L, seg.frameCount());
+                assertEquals("scan must stop at the header-backed/payload-unbacked frame",
+                        frame2Offset, seg.publishedOffset());
+                // Frame 2's header bytes are real (non-zero) and survive the
+                // truncate, so the bail-out region is flagged as a torn tail.
+                assertTrue("a torn write into the unbacked region must be flagged", seg.tornTailBytes() > 0);
             }
         });
     }
+
+    /**
+     * Creates a segment at {@code path} and appends one frame per entry in
+     * {@code payloadLens} (each filled with non-zero bytes so recovery can tell
+     * written data from an unwritten/zeroed hole). Returns the used byte count
+     * (the published offset after the last append).
+     */
+    private static long writeSegment(String path, long baseSeq, int[] payloadLens) {
+        int maxLen = 0;
+        for (int len : payloadLens) {
+            maxLen = Math.max(maxLen, len);
+        }
+        long buf = Unsafe.malloc(maxLen, MemoryTag.NATIVE_DEFAULT);
+        try {
+            for (int i = 0; i < maxLen; i++) {
+                Unsafe.getUnsafe().putByte(buf + i, (byte) (i | 1)); // all non-zero
+            }
+            try (MmapSegment seg = MmapSegment.create(path, baseSeq, SEGMENT_BYTES)) {
+                for (int len : payloadLens) {
+                    assertTrue("append must fit", seg.tryAppend(buf, len) >= 0);
+                }
+                return seg.publishedOffset();
+            }
+        } finally {
+            Unsafe.free(buf, maxLen, MemoryTag.NATIVE_DEFAULT);
+        }
+    }
+
+    /**
+     * Turns {@code [keepBytes, SEGMENT_BYTES)} of the file into an unbacked
+     * sparse hole: truncate down to {@code keepBytes} (frees the tail blocks),
+     * then back up to {@code SEGMENT_BYTES} (re-extends the logical size without
+     * allocating blocks). Recovery maps the full stat length, so the hole is
+     * inside the mapping -- reads of it fault on ZFS and zero-fill on ext4.
+     */
+    private static void punchSparseTail(String path, long keepBytes) {
+        int fd = Files.openRW(path);
+        assertTrue("openRW failed", fd >= 0);
+        try {
+            assertTrue("truncate down failed", Files.truncate(fd, keepBytes));
+            assertTrue("truncate up failed", Files.truncate(fd, SEGMENT_BYTES));
+        } finally {
+            Files.close(fd);
+        }
+    }
 }