Fix MVV marked-version corruption after interrupted prune

MVV.prune's finally-block safety net used the wrong loop bound
(index < length instead of index < offset + length), so a prune that
exited via an exception - e.g. PersistitInterruptedException while
resolving a commit status under interrupt - left mark bits set in the
live buffer page whenever the MVV sat at a non-zero in-page offset.
The fetch paths (visitAllVersions, fetchVersion, fetchVersionByOffset)
then read the version length signed and with the mark bit included,
driving the scan offset negative and crashing with
ArrayIndexOutOfBoundsException, as seen in
TransactionTest2.transactionsWithInterrupts on Windows JDK 11.

- Make the finally-block unmark loop in MVV.prune mirror the first
  pass exactly (bounded only by the array end, past which mark()
  cannot have written) and remove its early break on zero-length
  versions (a legal undefined value)
- Tighten the first pass's corruption guard so a misaligned traversal
  throws CorruptValueException before it can read or mark outside the
  MVV region
- Read version lengths unsigned with the mark bit stripped in
  visitAllVersions, fetchVersion and fetchVersionByOffset
- Throw CorruptValueException instead of AIOOBE when a version header
  or value extends past the MVV bounds
- Fix storeVersion reading target[0] instead of target[targetOffset]
  and exactRequiredLength's loop bound ignoring targetOffset when the
  MVV sits at a non-zero offset
- Add deterministic regression tests for marked-version reads and for
  the interrupted-prune unmark safety net; make
  transactionsWithInterrupts assert no worker died with an unexpected
  exception, reporting the first failure, with shared counters reset
  in @before

Fixes #286

Author: vharseko

persistit/core/src/main/java/com/persistit/MVV.java

@@ -171,7 +171,7 @@ static int exactRequiredLength(final byte[] target, final int targetOffset, fina
             return overheadLength(2) + targetLength + newVersionLength;
         } else {
             int offset = targetOffset + 1;
-            while (offset < targetLength) {
+            while (offset < targetOffset + targetLength) {
                 final long version = getVersion(target, offset);
                 final int valueLength = getLength(target, offset);
                 offset += LENGTH_PER_VERSION + valueLength;
@@ -295,7 +295,7 @@ else if (targetLength == 0) {
          * Value previously existed as a primordial value. Result will be an MVV
          * with two versions.
          */
-        else if (target[0] != TYPE_MVV_BYTE) {
+        else if (target[targetOffset] != TYPE_MVV_BYTE) {
             assertCapacity(targetLimit, targetOffset + sourceLength + targetLength + overheadLength(2));
             // Promote to MVV, shift existing down for header
             System.arraycopy(target, to, target, to + LENGTH_TYPE_MVV + LENGTH_PER_VERSION, targetLength);
@@ -484,7 +484,13 @@ static int prune(final byte[] bytes, final int offset, final int length, final T
                     }
                 }
                 from += vlength + LENGTH_PER_VERSION;
-                if (from > offset + length) {
+                if (from != offset + length && from + LENGTH_PER_VERSION > offset + length) {
+                    /*
+                     * A version must either end exactly at the region end or
+                     * leave room for at least one more version header. Anything
+                     * else is a misaligned traversal; reject it before the next
+                     * iteration reads or marks outside the MVV region.
+                     */
                     throw new CorruptValueException("MVV Value is corrupt at index: " + from);
                 }
             }
@@ -579,18 +585,22 @@ static int prune(final byte[] bytes, final int offset, final int length, final T
         } finally {
             /*
              * Make sure all marks are removed even if this method exits via an
-             * Exception.
+             * Exception. Marks left behind in a live buffer page are read back
+             * later as part of the version length and corrupt the MVV (see
+             * issue #286). A zero-length version is legal (undefined value),
+             * so the loop must not stop on vlength == 0; it always advances by
+             * at least LENGTH_PER_VERSION. The iteration must mirror the first
+             * pass exactly — on a corrupt MVV that pass can visit a version
+             * starting inside the trailing LENGTH_PER_VERSION bytes, so this
+             * loop may not stop earlier; the only hard bound is the array end,
+             * past which mark() could not have written.
              */
             if (marked > 0) {
                 int index = offset + 1;
-                while (index < length) {
+                while (index < offset + length && index + LENGTH_PER_VERSION <= bytes.length) {
                     final int vlength = getLength(bytes, index);
-                    Debug.$assert0.t(vlength + index + LENGTH_PER_VERSION <= offset + length);
                     unmark(bytes, index);
                     index += vlength + LENGTH_PER_VERSION;
-                    if (vlength <= 0) {
-                        break;
-                    }
                 }
             }
         }
@@ -663,8 +673,12 @@ static boolean verify(final TransactionIndex ti, final byte[] bytes, final int o
      *         if no value was copied.
      * @throws IllegalArgumentException
      *             If the target array is too small to hold the value
+     * @throws CorruptValueException
+     *             If a version header or value extends past
+     *             <code>sourceLength</code>
      */
-    public static int fetchVersion(final byte[] source, final int sourceLength, final long version, final byte[] target) {
+    public static int fetchVersion(final byte[] source, final int sourceLength, final long version, final byte[] target)
+            throws CorruptValueException {
         int offset = 0;
         int length = VERSION_NOT_FOUND;
 
@@ -673,9 +687,17 @@ public static int fetchVersion(final byte[] source, final int sourceLength, fina
         } else if (sourceLength > 0 && source[0] == TYPE_MVV_BYTE) {
             offset = 1;
             while (offset < sourceLength) {
-                final long curVersion = Util.getLong(source, offset);
-                final int curLength = Util.getShort(source, offset + LENGTH_VERSION);
+                if (offset + LENGTH_PER_VERSION > sourceLength) {
+                    throw new CorruptValueException("MVV version header at offset " + offset
+                            + " extends past length " + sourceLength);
+                }
+                final long curVersion = getVersion(source, offset);
+                final int curLength = getLength(source, offset);
                 offset += LENGTH_PER_VERSION;
+                if (curLength > sourceLength - offset) {
+                    throw new CorruptValueException("MVV version value at offset " + offset + " with length "
+                            + curLength + " extends past length " + sourceLength);
+                }
                 if (curVersion == version) {
                     length = curLength;
                     break;
@@ -739,15 +761,29 @@ public static void visitAllVersions(final VersionVisitor visitor, final byte[] s
         } else if (source[sourceOffset] != TYPE_MVV_BYTE) {
             visitor.sawVersion(PRIMORDIAL_VALUE_VERSION, sourceOffset, sourceLength);
         } else {
+            final int end = sourceOffset + sourceLength;
             int offset = sourceOffset + 1;
-            while (offset < sourceOffset + sourceLength) {
-                final long version = Util.getLong(source, offset);
-                final int valueLength = Util.getShort(source, offset + LENGTH_VERSION);
+            while (offset < end) {
+                if (offset + LENGTH_PER_VERSION > end) {
+                    throw new CorruptValueException("MVV version header at offset " + offset
+                            + " extends past offset/length=" + sourceOffset + "/" + sourceLength);
+                }
+                final long version = getVersion(source, offset);
+                final int valueLength = getLength(source, offset);
                 offset += LENGTH_PER_VERSION;
+                if (valueLength > end - offset) {
+                    throw new CorruptValueException("MVV version value at offset " + offset + " with length "
+                            + valueLength + " extends past offset/length=" + sourceOffset + "/" + sourceLength);
+                }
                 visitor.sawVersion(version, offset, valueLength);
                 offset += valueLength;
             }
-            if (offset != sourceOffset + sourceLength) {
+            /*
+             * Defensive only: the per-version guards above make this
+             * unreachable (the loop can only exit with offset == end). Kept in
+             * case the traversal logic changes.
+             */
+            if (offset != end) {
                 throw new CorruptValueException("invalid length in MVV at offset/length=" + sourceOffset + "/"
                         + sourceLength);
             }
@@ -773,14 +809,20 @@ public static void visitAllVersions(final VersionVisitor visitor, final byte[] s
      *         if no value was copied.
      * @throws IllegalArgumentException
      *             If the target array is too small to hold the value
+     * @throws CorruptValueException
+     *             If the stored value extends past <code>sourceLength</code>
      */
     public static int fetchVersionByOffset(final byte[] source, final int sourceLength, final int offset,
-            final byte[] target) {
+            final byte[] target) throws CorruptValueException {
         if (offset < 0 || (offset > 0 && offset > sourceLength)) {
             throw new IllegalArgumentException("Offset out of range: " + offset);
         }
-        final int length = (offset == 0) ? sourceLength : Util.getShort(source, offset - LENGTH_VALUE_LENGTH);
+        final int length = (offset == 0) ? sourceLength : getLength(source, offset - LENGTH_PER_VERSION);
         if (length > 0) {
+            if (length > sourceLength - offset) {
+                throw new CorruptValueException("MVV version value at offset " + offset + " with length " + length
+                        + " extends past length " + sourceLength);
+            }
             assertCapacity(target.length, length);
             System.arraycopy(source, offset, target, 0, length);
         }

persistit/core/src/test/java/com/persistit/MVVTest.java

@@ -12,15 +12,18 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
+ * Portions Copyrighted 2026 3A Systems, LLC
  */
 
 package com.persistit;
 
+import com.persistit.exception.CorruptValueException;
 import com.persistit.exception.PersistitException;
 import com.persistit.util.Util;
 import junit.framework.Assert;
 import org.junit.Test;
 
+import java.util.ArrayList;
 import java.util.Map;
 import java.util.TreeMap;
 
@@ -30,6 +33,7 @@
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class MVVTest {
     @Test
@@ -321,31 +325,31 @@ public void storeToExistingVersionAtCapacityShorterLength() {
     }
 
     @Test
-    public void fetchVersionFromUnused() {
+    public void fetchVersionFromUnused() throws PersistitException {
         final long vh = 10;
         final byte[] source = {};
         final byte[] target = {};
         assertEquals(MVV.VERSION_NOT_FOUND, MVV.fetchVersion(source, -1, vh, target));
     }
 
     @Test
-    public void fetchVersionFromUndefined() {
+    public void fetchVersionFromUndefined() throws PersistitException {
         final long vh = 10;
         final byte[] source = {};
         final byte[] target = {};
         assertEquals(MVV.VERSION_NOT_FOUND, MVV.fetchVersion(source, source.length, vh, target));
     }
 
     @Test
-    public void fetchVersionFromPrimordial() {
+    public void fetchVersionFromPrimordial() throws PersistitException {
         final long vh = 10;
         final byte[] source = { 0xA, 0xB, 0xC };
         final byte[] target = {};
         assertEquals(MVV.VERSION_NOT_FOUND, MVV.fetchVersion(source, source.length, vh, target));
     }
 
     @Test
-    public void fetchVersionFromExistingNoFound() {
+    public void fetchVersionFromExistingNoFound() throws PersistitException {
         final long vh = 10;
         final byte[] source = { (byte) TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 1, 0, 3, 0xA, 0xB, 0xC, 0, 0, 0, 0, 0, 0, 0, 2,
                 0, 2, 0xD, 0xE };
@@ -354,7 +358,7 @@ public void fetchVersionFromExistingNoFound() {
     }
 
     @Test
-    public void fetchVersionFromExisting() {
+    public void fetchVersionFromExisting() throws PersistitException {
         final long vh = 10;
         final byte[] source = { (byte) TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 10, 0, 2, 0xA, 0xB, 0, 0, 0, 0, 0, 0, 0, 11, 0,
                 3, 0xB, 0xC };
@@ -366,7 +370,7 @@ public void fetchVersionFromExisting() {
     }
 
     @Test(expected = IllegalArgumentException.class)
-    public void fetchVersionFromExistingOverCapacity() {
+    public void fetchVersionFromExistingOverCapacity() throws PersistitException {
         final long vh = 10;
         final byte[] source = { (byte) TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 11, 0, 5, 0x1, 0x2, 0x3, 0x4, 0x5, 0, 0, 0, 0, 0,
                 0, 0, 9, 0, 1, 0xA, 0, 0, 0, 0, 0, 0, 0, 10, 0, 3, 0xB, 0xC, 0xD };
@@ -429,21 +433,21 @@ public void visitAndFetchByOffsetMVV() throws PersistitException {
     }
 
     @Test(expected = IllegalArgumentException.class)
-    public void fetchByOffsetNegative() {
+    public void fetchByOffsetNegative() throws PersistitException {
         final byte[] source = {};
         final byte[] target = new byte[10];
         MVV.fetchVersionByOffset(source, source.length, -1, target);
     }
 
     @Test(expected = IllegalArgumentException.class)
-    public void fetchByOffsetTooLarge() {
+    public void fetchByOffsetTooLarge() throws PersistitException {
         final byte[] source = newArray(TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 1, 0, 3, 0xA, 0xB, 0xC);
         final byte[] target = new byte[10];
         MVV.fetchVersionByOffset(source, source.length, source.length + 1, target);
     }
 
     @Test
-    public void storeAndFetchVersionMany() {
+    public void storeAndFetchVersionMany() throws PersistitException {
         final int VERSION_COUNT = 10;
         final int versions[] = new int[VERSION_COUNT];
         final byte sources[][] = new byte[VERSION_COUNT][];
@@ -480,6 +484,109 @@ public void tryValueTooLong() {
         MVV.storeVersion(target, 0, 0, target.length, VERSION, source, 0, source.length);
     }
 
+    //
+    // Issue #286: a version left in the marked state (e.g. by a prune that was
+    // interrupted mid-way) must still be readable. The length accessors used
+    // by the fetch paths formerly read the length field signed and with the
+    // mark bit included, driving the scan offset negative and throwing
+    // ArrayIndexOutOfBoundsException.
+    //
+
+    @Test
+    public void visitMarkedVersion() throws PersistitException {
+        final byte[] source = newArray(TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 1, 0, 3, 0xA, 0xB, 0xC, 0, 0, 0, 0, 0, 0, 0, 2,
+                0, 2, 0xD, 0xE);
+        MVV.mark(source, 1);
+        final TestVisitor visitor = new TestVisitor();
+        MVV.visitAllVersions(visitor, source, 0, source.length);
+        assertTrue(visitor.initCalled);
+        assertEquals(newVisitorMap(1, 3, 11, 2, 2, 24), visitor.versions);
+    }
+
+    @Test
+    public void fetchMarkedVersion() throws PersistitException {
+        final byte[] source = newArray(TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 10, 0, 2, 0xA, 0xB, 0, 0, 0, 0, 0, 0, 0, 11, 0,
+                3, 0xC, 0xD, 0xE);
+        MVV.mark(source, 1);
+        MVV.mark(source, 13);
+        final byte[] target = new byte[20];
+        assertEquals(2, MVV.fetchVersion(source, source.length, 10, target));
+        assertArrayEqualsLen(newArray(0xA, 0xB), target, 2);
+        assertEquals(3, MVV.fetchVersion(source, source.length, 11, target));
+        assertArrayEqualsLen(newArray(0xC, 0xD, 0xE), target, 3);
+    }
+
+    @Test
+    public void fetchByOffsetMarkedVersion() throws PersistitException {
+        final byte[] source = newArray(TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 10, 0, 2, 0xA, 0xB, 0, 0, 0, 0, 0, 0, 0, 11, 0,
+                3, 0xC, 0xD, 0xE);
+        MVV.mark(source, 1);
+        final byte[] target = new byte[20];
+        assertEquals(2, MVV.fetchVersionByOffset(source, source.length, 11, target));
+        assertArrayEqualsLen(newArray(0xA, 0xB), target, 2);
+    }
+
+    @Test(expected = CorruptValueException.class)
+    public void visitTruncatedHeaderThrows() throws PersistitException {
+        final byte[] source = newArray(TYPE_MVV, 0, 0, 0, 0, 0);
+        MVV.visitAllVersions(new TestVisitor(), source, 0, source.length);
+    }
+
+    /**
+     * The old code also threw CorruptValueException here (from the trailing
+     * length check), but only after handing the visitor an out-of-bounds
+     * version — which Exchange.MvvVisitor would record and fetchVersionByOffset
+     * would then copy. The fix must throw before the visitor sees it.
+     */
+    @Test
+    public void visitOverrunningLengthThrows() throws PersistitException {
+        final byte[] source = newArray(TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 1, 0, 50, 0xA, 0xB, 0xC);
+        final TestVisitor visitor = new TestVisitor();
+        try {
+            MVV.visitAllVersions(visitor, source, 0, source.length);
+            fail("expected CorruptValueException");
+        } catch (final CorruptValueException expected) {
+        }
+        assertTrue("visitor must not see an out-of-bounds version", visitor.versions.isEmpty());
+    }
+
+    /**
+     * Issue #286: when prune exits via an exception after its first pass has
+     * marked versions (e.g. interrupted while resolving a commit status, or a
+     * CorruptValueException), the finally-block safety net must remove the
+     * marks. Its loop bound was wrong for a non-zero offset — exactly the
+     * in-page pruning case — leaving mark bits behind in the live buffer page.
+     */
+    @Test
+    public void pruneExceptionUnmarksVersionsAtNonZeroOffset() throws Exception {
+        final TimestampAllocator tsa = new TimestampAllocator();
+        final TransactionIndex ti = new TransactionIndex(tsa, 1);
+        final TransactionStatus status1 = ti.registerTransaction();
+        final TransactionStatus status2 = ti.registerTransaction();
+
+        /*
+         * Two uncommitted versions from different transactions make prune's
+         * first pass throw "Multiple uncommitted versions" after it has marked
+         * the first version. The non-zero offset emulates in-page pruning.
+         */
+        final int offset = 117;
+        final byte[] bytes = new byte[offset + 100];
+        final byte[] v1 = { 0xA, 0xB, 0xC };
+        final byte[] v2 = { 0xD, 0xE };
+        int length = MVV.storeVersion(bytes, offset, -1, bytes.length, TransactionIndex.ts2vh(status1.getTs()), v1, 0,
+                v1.length) & STORE_LENGTH_MASK;
+        length = MVV.storeVersion(bytes, offset, length, bytes.length, TransactionIndex.ts2vh(status2.getTs()), v2, 0,
+                v2.length) & STORE_LENGTH_MASK;
+
+        final byte[] before = bytes.clone();
+        try {
+            MVV.prune(bytes, offset, length, ti, true, new ArrayList<MVV.PrunedVersion>());
+            fail("expected CorruptValueException");
+        } catch (final CorruptValueException expected) {
+        }
+        assertArrayEquals("prune must leave the byte array unchanged when it throws", before, bytes);
+    }
+
     //
     // Test helper methods
     //