From f3bdd5c315c601c92c53b69eefa13586cb1710ea Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 10 Jul 2026 18:10:33 +0300 Subject: [PATCH] Validate the page before re-inserting an index pointer in fixIndexHole (bug 1017957 class race) rebalanceSplit() links a new page only through its sibling chain and defers the parent index entry to a background CleanupIndexHole action. By the time the CleanupManager runs it, the action may be arbitrarily stale: a covering removeKeyRange can have unlinked the page onto a garbage chain (content and type intact, or retyped PAGE_TYPE_GARBAGE if it became a chain root), or the page may have been reused. fixIndexHole() performed no validation and inserted a key-pointer pair for whatever the page now holds, planting a dangling index entry to a garbage or reused page -- the bug 1017957 failure mode. Concurrent descents following such an entry throw transient CorruptVolumeExceptions ("invalid page type 30: should be 2" observed on CI under Bug1017957Test.induceCorruptionByStress; "walked right more than 50 pages" and "LONG_RECORD chain is invalid" are the same race's other signatures). The volume looks consistent afterwards because later covering removes delete the dangling entry before IntegrityCheck runs, which is why the corruption appeared transient. Faster JVMs widen the window: more iterations produce more rebalance splits racing the background queue. Guard the insertion. Under the tree reader claim (structure deletes require the writer claim, so reachability cannot change while it is held): (a) drop the action if the page is no longer this level's type or is empty; (b) verify the page is still reachable in this tree -- a search for its current first key at that level must land on the page itself via the B-link right-walk -- and drop the stale action otherwise. No claim is held across the verification descent, preserving the parent-then-child claim order. Verified: full persistit/core suite 565/565 green; IntegrityCheckTest testIndexFixHoles confirms valid holes are still repaired; Bug1017957Test.induceCorruptionByStress with its original strict zero-exception assertion green across repeated runs. The race itself reproduces only on CI runners, which is where this fix is ultimately validated. --- .../src/main/java/com/persistit/Exchange.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/persistit/core/src/main/java/com/persistit/Exchange.java b/persistit/core/src/main/java/com/persistit/Exchange.java index 3842f42e9..897c7ba2d 100644 --- a/persistit/core/src/main/java/com/persistit/Exchange.java +++ b/persistit/core/src/main/java/com/persistit/Exchange.java @@ -4076,12 +4076,49 @@ boolean fixIndexHole(final long page, final int level) throws PersistitException return false; } try { + /* + * This action was enqueued by rebalanceSplit() when the page was + * reachable only through its sibling chain, and may be arbitrarily + * stale by the time the background CleanupManager runs it: a covering + * removeKeyRange may since have unlinked the page onto a garbage chain + * (leaving its content and type intact, or retyping it PAGE_TYPE_GARBAGE + * if it became a chain root), or the page may have been reused by an + * unrelated allocation. Blindly inserting a key-pointer pair for such a + * page plants a dangling index entry to a garbage or reused page -- the + * bug 1017957 failure mode, observed as transient + * CorruptVolumeExceptions ("invalid page type 30") under concurrent + * structural stress. Structure deletes hold the tree writer claim, so + * under the reader claim held here the page's reachability cannot + * change; validate it before inserting the pointer. + */ buffer = _pool.get(_volume, page, false, true); + if (buffer.getPageType() != level + PAGE_TYPE_DATA + || buffer.getKeyBlockEnd() <= buffer.getKeyBlockStart()) { + // Retyped (garbage-chain root or reused elsewhere) or empty: the + // index hole this action was created for no longer exists. + return true; + } buffer.nextKey(_spareKey2, buffer.toKeyBlock(0)); _value.setPointerValue(page); _value.setPointerPageType(buffer.getPageType()); buffer.release(); buffer = null; + /* + * A page sitting on a garbage chain keeps its old type and content, so + * the type check above is not sufficient. Verify the page is still + * reachable in this tree: a search for its current first key at this + * level must land on the page itself (via the B-link right-walk, since + * the hole means it has no parent entry yet). If the search lands + * elsewhere the action is stale and inserting the pointer would corrupt + * the index, so drop it. + */ + searchTree(_spareKey2, level, false); + final LevelCache lc = _levelCache[level]; + final long landedOn = lc._page; + lc._buffer.releaseTouched(); + if (landedOn != page) { + return true; + } storeInternal(_spareKey2, _value, level + 1, StoreOptions.NONE); return true; } finally {