From 68b642a7f80df960bcd43aabf62aa1dafc3e3891 Mon Sep 17 00:00:00 2001 From: Dreeam <61569423+Dreeam-qwq@users.noreply.github.com> Date: Thu, 17 Apr 2025 18:44:13 -0400 Subject: [PATCH] Fix race condition in IteratorSafeOrderedReferenceSet (#278) * Fix race condition in IteratorSafeOrderedReferenceSet * Use cached firstInvalidIndex at the beginning of the method --- ...004-Pufferfish-Optimize-mob-spawning.patch | 96 +++++++++++++++++-- 1 file changed, 90 insertions(+), 6 deletions(-) diff --git a/leaf-server/paper-patches/features/0004-Pufferfish-Optimize-mob-spawning.patch b/leaf-server/paper-patches/features/0004-Pufferfish-Optimize-mob-spawning.patch index 345bc1c6..e9697ac3 100644 --- a/leaf-server/paper-patches/features/0004-Pufferfish-Optimize-mob-spawning.patch +++ b/leaf-server/paper-patches/features/0004-Pufferfish-Optimize-mob-spawning.patch @@ -23,7 +23,7 @@ and, in my opinion, worth the low risk of minor mob-spawning-related inconsistencies. diff --git a/src/main/java/ca/spottedleaf/moonrise/common/list/IteratorSafeOrderedReferenceSet.java b/src/main/java/ca/spottedleaf/moonrise/common/list/IteratorSafeOrderedReferenceSet.java -index c21e00812f1aaa1279834a0562d360d6b89e146c..4ae478c04ef44c91408a7f3f0405291f91794873 100644 +index c21e00812f1aaa1279834a0562d360d6b89e146c..19e0bf50cf8863f42ce19c3d0d8911c0c9b8ad4a 100644 --- a/src/main/java/ca/spottedleaf/moonrise/common/list/IteratorSafeOrderedReferenceSet.java +++ b/src/main/java/ca/spottedleaf/moonrise/common/list/IteratorSafeOrderedReferenceSet.java @@ -10,7 +10,7 @@ public final class IteratorSafeOrderedReferenceSet { @@ -31,7 +31,7 @@ index c21e00812f1aaa1279834a0562d360d6b89e146c..4ae478c04ef44c91408a7f3f0405291f private final Reference2IntLinkedOpenHashMap indexMap; - private int firstInvalidIndex = -1; -+ private volatile int firstInvalidIndex = -1; // Leaf - Async mob spawning - volatile ++ private final java.util.concurrent.atomic.AtomicInteger firstInvalidIndex = new java.util.concurrent.atomic.AtomicInteger(-1); // Leaf - Pufferfish - Async mob spawning - atomic /* list impl */ private E[] listElements; @@ -44,7 +44,22 @@ index c21e00812f1aaa1279834a0562d360d6b89e146c..4ae478c04ef44c91408a7f3f0405291f public IteratorSafeOrderedReferenceSet() { this(16, 0.75f, 16, 0.2); -@@ -79,7 +79,7 @@ public final class IteratorSafeOrderedReferenceSet { +@@ -74,16 +74,26 @@ public final class IteratorSafeOrderedReferenceSet { + } + */ + ++ // Pufferfish start - async mob spawning ++ // TODO: backup plan if the issue persists, don't run ++ // this.set.finishRawIterator() or defrag() off-main ++ /* ++ protected final boolean allowSafeIteration() { ++ return ca.spottedleaf.moonrise.common.util.TickThread.isTickThread(); ++ } ++ */ ++ // Pufferfish end - async mob spawning ++ + private double getFragFactor() { + return 1.0 - ((double)this.indexMap.size() / (double)this.listSize); } public int createRawIterator() { @@ -53,7 +68,12 @@ index c21e00812f1aaa1279834a0562d360d6b89e146c..4ae478c04ef44c91408a7f3f0405291f if (this.indexMap.isEmpty()) { return -1; } else { -@@ -100,7 +100,7 @@ public final class IteratorSafeOrderedReferenceSet { +- return this.firstInvalidIndex == 0 ? this.indexMap.getInt(this.indexMap.firstKey()) : 0; ++ return this.firstInvalidIndex.get() == 0 ? this.indexMap.getInt(this.indexMap.firstKey()) : 0; // Leaf - Pufferfish - Async mob spawning + } + } + +@@ -100,7 +110,7 @@ public final class IteratorSafeOrderedReferenceSet { } public void finishRawIterator() { @@ -62,7 +82,19 @@ index c21e00812f1aaa1279834a0562d360d6b89e146c..4ae478c04ef44c91408a7f3f0405291f if (this.getFragFactor() >= this.maxFragFactor) { this.defrag(); } -@@ -117,7 +117,7 @@ public final class IteratorSafeOrderedReferenceSet { +@@ -110,14 +120,17 @@ public final class IteratorSafeOrderedReferenceSet { + public boolean remove(final E element) { + final int index = this.indexMap.removeInt(element); + if (index >= 0) { +- if (this.firstInvalidIndex < 0 || index < this.firstInvalidIndex) { +- this.firstInvalidIndex = index; ++ // Leaf start - Pufferfish - Async mob spawning ++ int firstInvalidIndex = this.firstInvalidIndex.get(); ++ if (firstInvalidIndex < 0 || index < firstInvalidIndex) { ++ this.firstInvalidIndex.set(index); + } ++ // Leaf end - Pufferfish - Async mob spawning + if (this.listElements[index] != element) { throw new IllegalStateException(); } this.listElements[index] = null; @@ -71,7 +103,50 @@ index c21e00812f1aaa1279834a0562d360d6b89e146c..4ae478c04ef44c91408a7f3f0405291f this.defrag(); } //this.check(); -@@ -219,7 +219,7 @@ public final class IteratorSafeOrderedReferenceSet { +@@ -149,14 +162,17 @@ public final class IteratorSafeOrderedReferenceSet { + } + + private void defrag() { +- if (this.firstInvalidIndex < 0) { ++ // Leaf start - Pufferfish - Async mob spawning ++ int firstInvalidIndex = this.firstInvalidIndex.get(); ++ if (firstInvalidIndex < 0) { + return; // nothing to do + } ++ // Leaf end - Pufferfish - Async mob spawning + + if (this.indexMap.isEmpty()) { + Arrays.fill(this.listElements, 0, this.listSize, null); + this.listSize = 0; +- this.firstInvalidIndex = -1; ++ this.firstInvalidIndex.set(-1); // Leaf - Pufferfish - Async mob spawning + //this.check(); + return; + } +@@ -166,11 +182,11 @@ public final class IteratorSafeOrderedReferenceSet { + int lastValidIndex; + java.util.Iterator> iterator; + +- if (this.firstInvalidIndex == 0) { ++ if (firstInvalidIndex == 0) { // Leaf - Pufferfish - Async mob spawning + iterator = this.indexMap.reference2IntEntrySet().fastIterator(); + lastValidIndex = 0; + } else { +- lastValidIndex = this.firstInvalidIndex; ++ lastValidIndex = firstInvalidIndex; // Leaf - Pufferfish - Async mob spawning + final E key = backingArray[lastValidIndex - 1]; + iterator = this.indexMap.reference2IntEntrySet().fastIterator(new Reference2IntMap.Entry() { + @Override +@@ -201,7 +217,7 @@ public final class IteratorSafeOrderedReferenceSet { + // cleanup end + Arrays.fill(backingArray, lastValidIndex, this.listSize, null); + this.listSize = lastValidIndex; +- this.firstInvalidIndex = -1; ++ this.firstInvalidIndex.set(-1); // Leaf - Pufferfish - Async mob spawning + //this.check(); + } + +@@ -219,7 +235,7 @@ public final class IteratorSafeOrderedReferenceSet { } public IteratorSafeOrderedReferenceSet.Iterator iterator(final int flags) { @@ -80,3 +155,12 @@ index c21e00812f1aaa1279834a0562d360d6b89e146c..4ae478c04ef44c91408a7f3f0405291f return new BaseIterator<>(this, true, (flags & ITERATOR_FLAG_SEE_ADDITIONS) != 0 ? Integer.MAX_VALUE : this.listSize); } +@@ -306,7 +322,7 @@ public final class IteratorSafeOrderedReferenceSet { + } + this.lastReturned = null; + this.finished = true; +- this.set.finishRawIterator(); ++ this.set.finishRawIterator(); // Pufferfish - async mob spawning - diff on change + } + } + }