diff --git a/src/main/java/net/openhft/chronicle/hash/impl/stage/entry/SegmentStages.java b/src/main/java/net/openhft/chronicle/hash/impl/stage/entry/SegmentStages.java index e9f5d016a..c2869d223 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/stage/entry/SegmentStages.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/stage/entry/SegmentStages.java @@ -655,4 +655,28 @@ public abstract class SegmentStages implements SegmentLock, LocksInterface { nextPosToSearchFrom = 0L; nextPosToSearchFrom(nextPosToSearchFrom); } + + public void verifyTierCountersAreaData() { + goToFirstTier(); + while (true) { + int tierSegmentIndex = TierCountersArea.segmentIndex(tierCountersAreaAddr()); + if (tierSegmentIndex != segmentIndex) { + throw new AssertionError("segmentIndex: " + segmentIndex + + ", tier: " + tier + ", tierIndex: " + tierIndex + ", tierBaseAddr: " + + tierBaseAddr + " reports it belongs to segmentIndex " + tierSegmentIndex); + } + if (hasNextTier()) { + long currentTierIndex = this.tierIndex; + nextTier(); + if (prevTierIndex() != currentTierIndex) { + throw new AssertionError("segmentIndex: " + segmentIndex + + ", tier: " + tier + ", tierIndex: " + tierIndex + ", tierBaseAddr: " + + tierBaseAddr + " reports the previous tierIndex is " + prevTierIndex() + + " while actually it is " + currentTierIndex); + } + } else { + break; + } + } + } } diff --git a/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/HashSegmentIteration.java b/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/HashSegmentIteration.java index 2a6c938cd..6cc36f5c3 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/HashSegmentIteration.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/stage/iter/HashSegmentIteration.java @@ -103,12 +103,14 @@ public abstract class HashSegmentIteration> } hlp.initHashLookupPos(startPos); long currentHashLookupPos; + int steps = 0; do { // Step from hlp.hashLookupPos, not currentHashLookupPos (with additional initialization // of this local variable to startPos outside the loop), because if e.remove() is // called in the `action`, hlp.hashLookupPos is stepped back in doRemove(), and // currentHashLookupPos become invalid currentHashLookupPos = hashLookup.step(hlp.hashLookupPos); + steps++; hlp.setHashLookupPos(currentHashLookupPos); long entry = hashLookup.readEntry(currentTierBaseAddr, currentHashLookupPos); initHashLookupEntry(entry); @@ -135,12 +137,17 @@ public abstract class HashSegmentIteration> // slot in _current_ tier's hash lookup is shift deleted, see // relocation() currentHashLookupPos = hashLookup.stepBack(currentHashLookupPos); + steps--; hlp.initHashLookupPos(currentHashLookupPos); } } } } - } while (currentHashLookupPos != startPos); + // the `steps == 0` condition and this variable updates in the loop fix the bug, when + // shift deletion occurs on the first entry of the tier, and the currentHashLookupPos + // becomes equal to start pos without making the whole loop, but only visiting a single + // entry + } while (currentHashLookupPos != startPos || steps == 0); return interrupted ? ~size : size; } diff --git a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java index 9ba6313fc..6432e72a7 100755 --- a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java +++ b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java @@ -38,6 +38,8 @@ import org.jetbrains.annotations.NotNull; import java.io.IOException; import java.io.ObjectInputStream; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; @@ -464,4 +466,16 @@ public class VanillaChronicleMap return q.defaultReturnValue().returnValue(); } } + + public void verifyTierCountersAreaData() { + for (int i = 0; i < actualSegments; i++) { + try (MapSegmentContext c = segmentContext(i)) { + Method verifyTierCountersAreaData = + c.getClass().getMethod("verifyTierCountersAreaData"); + verifyTierCountersAreaData.invoke(c); + } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { + throw new AssertionError(e); + } + } + } } diff --git a/src/test/java/net/openhft/chronicle/map/OverflowAllocationDuringIterationTest.java b/src/test/java/net/openhft/chronicle/map/OverflowAllocationDuringIterationTest.java index ec9c57237..a8117a8a1 100644 --- a/src/test/java/net/openhft/chronicle/map/OverflowAllocationDuringIterationTest.java +++ b/src/test/java/net/openhft/chronicle/map/OverflowAllocationDuringIterationTest.java @@ -35,19 +35,23 @@ public class OverflowAllocationDuringIterationTest { map.put(i, x); } for (int i = 0; i < 3; i++) { - final String currentX = x; - map.forEachEntry(e -> { - String v = e.value().get().toString(); - if (!currentX.contentEquals(v)) { - throw new AssertionError(currentX + " != " + v + "<=" + e.key()); - } - e.doReplaceValue(e.context().wrapValueAsData(v + v)); - }); - x = x + x; - for (int j = 0; j < entries; j++) { - if (map.get(j) == null || !x.contentEquals(map.get(j))) { - throw new AssertionError(); + try { + final String currentX = x; + map.forEachEntry(e -> { + String v = e.value().get().toString(); + if (!currentX.contentEquals(v)) { + throw new AssertionError(currentX + " != " + v + "<=" + e.key()); + } + e.doReplaceValue(e.context().wrapValueAsData(v + v)); + }); + x = x + x; + for (int j = 0; j < entries; j++) { + if (map.get(j) == null || !x.contentEquals(map.get(j))) { + throw new AssertionError(); + } } + } finally { + ((VanillaChronicleMap) map).verifyTierCountersAreaData(); } } }