Skip a segment whose mmap fault arrives late

SegmentRing's per-file arm caught only MmapSegmentException, so an
unbacked or sparse mapped page in one .sfa could abort recovery of the
whole slot and strand every frame the sibling segments still held.

MmapSegment installs handlers around its own recovery reads, but they
cannot be relied on before JDK 21: HotSpot records an unsafe-access
fault and raises the InternalError at the next return or safepoint
check rather than at the faulting read, so it surfaces in an arbitrary
caller frame. JDK-8283699 made delivery precise only in 21+. Observed
on 8 and 17 landing two frames above SegmentRing.openExisting, past
every handler MmapSegment installs -- which is what left the two
map-past-EOF guards in MmapSegmentRecoveryFaultTest erroring on the
JDK 8 job while passing on 25.

Convert at the per-file boundary instead, the frame the late delivery
lands in. The arm widens to Throwable but rethrows anything that is
neither MmapSegmentException nor the recognized mmap access fault, so
resource exhaustion and programmer errors still propagate to the outer
catch as before. isMmapAccessFault becomes package-private for that
caller and carries the delivery caveat in its javadoc.

Add a facade-aware SegmentRing.openExisting overload, mirroring the
seam MmapSegment already exposes, and two tests that inject the fault
through it deterministically on any JDK: one asserting the faulting
segment is skipped while its sibling's frames survive, one asserting an
unrecognized InternalError still propagates so the widened catch cannot
degrade into skip-on-anything. The two pre-existing map-past-EOF tests
now drive SegmentRing -- the boundary their own javadocs describe --
keeping the strong assertion where delivery is precise and degrading on
pre-21 to what the JVM still makes checkable: the process survived and
the escaping error is the recognized fault.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: glasstiger

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

@@ -649,8 +649,23 @@ public long tornTailBytes() {
      * 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.
+     * <p>
+     * <b>Delivery is NOT precise before JDK 21, so callers must guard too.</b>
+     * Pre-21 HotSpot records the fault and raises the {@code InternalError} at
+     * the next return or safepoint check rather than at the faulting
+     * instruction, so it can surface in a CALLER's frame -- past every handler
+     * in this class. Observed on JDK 8/17: a fault taken inside
+     * {@link #scanFrames} arrives only after {@link #openExisting} has already
+     * returned its segment, so neither the scan's own catch nor
+     * {@code openExisting}'s outer catch ever sees it. JDK-8283699 made
+     * delivery precise in 21+, which is why the same case is fully contained
+     * there. The in-class catches therefore handle the common (precise) case
+     * only; {@link SegmentRing#openExisting} applies this same predicate at the
+     * per-file boundary so a late-delivered fault still skips one {@code .sfa}
+     * instead of aborting recovery of the whole slot. Package-private for that
+     * caller.
      */
-    private static boolean isMmapAccessFault(Throwable t) {
+    static boolean isMmapAccessFault(Throwable t) {
         if (!(t instanceof InternalError)) {
             return false;
         }

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

@@ -25,6 +25,7 @@
 package io.questdb.client.cutlass.qwp.client.sf.cursor;
 
 import io.questdb.client.std.Files;
+import io.questdb.client.std.FilesFacade;
 import io.questdb.client.std.ObjList;
 import io.questdb.client.std.QuietCloseable;
 import org.jetbrains.annotations.TestOnly;
@@ -168,6 +169,21 @@ public SegmentRing(MmapSegment initialActive, long maxBytesPerSegment) {
      * depends on every segment being readable.
      */
     public static SegmentRing openExisting(String sfDir, long maxBytesPerSegment) {
+        return openExisting(FilesFacade.INSTANCE, sfDir, maxBytesPerSegment);
+    }
+
+    /**
+     * Facade-aware variant of {@link #openExisting(String, long)}. Only the
+     * per-segment {@link MmapSegment#openExisting(FilesFacade, String)} call
+     * takes the facade -- directory enumeration and the empty-segment unlink
+     * stay on {@link Files}, because the seam exists for one purpose: letting a
+     * facade report an inflated stat length so a segment maps past
+     * end-of-file and its recovery reads fault deterministically on ANY
+     * filesystem. That is what regression-tests the per-file mmap-fault skip
+     * below, which on a real deployment only triggers on a sparse
+     * pre-allocation tail (e.g. ZFS) or a file truncated under the mapping.
+     */
+    public static SegmentRing openExisting(FilesFacade ff, String sfDir, long maxBytesPerSegment) {
         if (!Files.exists(sfDir)) {
             return null;
         }
@@ -197,7 +213,7 @@ public static SegmentRing openExisting(String sfDir, long maxBytesPerSegment) {
                         String path = sfDir + "/" + name;
                         MmapSegment seg = null;
                         try {
-                            seg = MmapSegment.openExisting(path);
+                            seg = MmapSegment.openExisting(ff, path);
                             // Filter out empty leftovers -- typically hot-spare
                             // segments the manager pre-allocated for a prior
                             // session that never got rotated into active. They
@@ -231,7 +247,7 @@ public static SegmentRing openExisting(String sfDir, long maxBytesPerSegment) {
                                 opened.add(seg);
                                 seg = null;
                             }
-                        } catch (MmapSegmentException t) {
+                        } catch (Throwable t) {
                             // Per-file data error (bad magic, bad header,
                             // unsupported version, mmap rejection on this one
                             // file). Don't take down recovery for one corrupt
@@ -243,6 +259,24 @@ public static SegmentRing openExisting(String sfDir, long maxBytesPerSegment) {
                             // an OOM would just fail again on the next file
                             // while silently leaking the segments we managed
                             // to recover before it.
+                            //
+                            // The mmap-access-fault arm catches Throwable
+                            // rather than just MmapSegmentException because
+                            // pre-21 HotSpot delivers an unsafe-access fault
+                            // ASYNCHRONOUSLY -- the InternalError can surface
+                            // here, in MmapSegment.openExisting's caller,
+                            // instead of inside the handlers that class
+                            // installs around its own reads (see
+                            // MmapSegment.isMmapAccessFault). An unbacked or
+                            // sparse page in ONE segment must still skip only
+                            // that segment; letting the raw InternalError reach
+                            // the outer catch would abort recovery of the whole
+                            // slot and strand every frame the other segments
+                            // still hold.
+                            if (!(t instanceof MmapSegmentException)
+                                    && !MmapSegment.isMmapAccessFault(t)) {
+                                throw t;
+                            }
                             LOG.warn("openExisting: skipping {} -- {}", path, t.toString());
                         } finally {
                             // Close any seg whose ownership wasn't transferred