feat: remove char block sections array

- replace with "forced" memory access order to eliminate race conditions between sections and blocks arrays
IntellectualSites · Feb 9, 2025 · 35f24eb · 35f24eb
1 parent 9380555
commit 35f24eb
Show file tree

Hide file tree

Showing 11 changed files with 76 additions and 99 deletions.
diff --git a/...main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R2/PaperweightGetBlocks.java b/...main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R2/PaperweightGetBlocks.java
@@ -1016,7 +1016,7 @@ public synchronized boolean trim(boolean aggressive) {
         } else {
             for (int i = getMinSectionPosition(); i <= getMaxSectionPosition(); i++) {
                 int layer = i - getMinSectionPosition();
-                if (!hasSection(i) || !super.sections[layer].isFull()) {
+                if (!hasSection(i) || super.blocks[layer] == null) {
                     continue;
                 }
                 LevelChunkSection existing = getSections(true)[layer];

diff --git a/...main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R3/PaperweightGetBlocks.java b/...main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R3/PaperweightGetBlocks.java
@@ -1017,7 +1017,7 @@ public synchronized boolean trim(boolean aggressive) {
         } else {
             for (int i = getMinSectionPosition(); i <= getMaxSectionPosition(); i++) {
                 int layer = i - getMinSectionPosition();
-                if (!hasSection(i) || !super.sections[layer].isFull()) {
+                if (!hasSection(i) || super.blocks[layer] == null) {
                     continue;
                 }
                 LevelChunkSection existing = getSections(true)[layer];

diff --git a/...main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R4/PaperweightGetBlocks.java b/...main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R4/PaperweightGetBlocks.java
@@ -1018,7 +1018,7 @@ public synchronized boolean trim(boolean aggressive) {
         } else {
             for (int i = getMinSectionPosition(); i <= getMaxSectionPosition(); i++) {
                 int layer = i - getMinSectionPosition();
-                if (!hasSection(i) || !super.sections[layer].isFull()) {
+                if (!hasSection(i) || super.blocks[layer] == null) {
                     continue;
                 }
                 LevelChunkSection existing = getSections(true)[layer];

diff --git a/...main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_R1/PaperweightGetBlocks.java b/...main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_R1/PaperweightGetBlocks.java
@@ -1052,7 +1052,7 @@ public synchronized boolean trim(boolean aggressive) {
         } else {
             for (int i = getMinSectionPosition(); i <= getMaxSectionPosition(); i++) {
                 int layer = i - getMinSectionPosition();
-                if (!hasSection(i) || !super.sections[layer].isFull()) {
+                if (!hasSection(i) || super.blocks[layer] == null) {
                     continue;
                 }
                 LevelChunkSection existing = getSections(true)[layer];

diff --git a/.../main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_3/PaperweightGetBlocks.java b/.../main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_3/PaperweightGetBlocks.java
@@ -1011,7 +1011,7 @@ public synchronized boolean trim(boolean aggressive) {
         } else {
             for (int i = getMinSectionPosition(); i <= getMaxSectionPosition(); i++) {
                 int layer = i - getMinSectionPosition();
-                if (!hasSection(i) || !super.sections[layer].isFull()) {
+                if (!hasSection(i) || super.blocks[layer] == null) {
                     continue;
                 }
                 LevelChunkSection existing = getSections(true)[layer];

diff --git a/.../main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_4/PaperweightGetBlocks.java b/.../main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_4/PaperweightGetBlocks.java
@@ -1013,7 +1013,7 @@ public synchronized boolean trim(boolean aggressive) {
         } else {
             for (int i = getMinSectionPosition(); i <= getMaxSectionPosition(); i++) {
                 int layer = i - getMinSectionPosition();
-                if (!hasSection(i) || !super.sections[layer].isFull()) {
+                if (!hasSection(i) || super.blocks[layer] == null) {
                     continue;
                 }
                 LevelChunkSection existing = getSections(true)[layer];

diff --git a/...t-core/src/main/java/com/fastasyncworldedit/core/extent/filter/block/CharFilterBlock.java b/...t-core/src/main/java/com/fastasyncworldedit/core/extent/filter/block/CharFilterBlock.java
@@ -354,8 +354,7 @@ public final BlockState getBlockBelow() {
         }
         if (layer > minLayer) {
             final int newLayer = layer - 1;
-            final CharGetBlocks chunk = this.get;
-            return states[chunk.sections[newLayer].get(chunk, newLayer, index + 3840)];
+            return states[get.get(newLayer, index + 3840)];
         }
         return BlockTypes.__RESERVED__.getDefaultState();
     }
@@ -367,8 +366,7 @@ public final BlockState getBlockAbove() {
         }
         if (layer < maxLayer) {
             final int newLayer = layer + 1;
-            final CharGetBlocks chunk = this.get;
-            return states[chunk.sections[newLayer].get(chunk, newLayer, index - 3840)];
+            return states[get.get(newLayer, index - 3840)];
         }
         return BlockTypes.__RESERVED__.getDefaultState();
     }
@@ -382,7 +380,7 @@ public final BlockState getBlockRelativeY(int y) {
         } else if ((layerAdd > 0 && layerAdd < (maxLayer - layer)) || (layerAdd < 0 && layerAdd < (minLayer - layer))) {
             final int newLayer = layer + layerAdd;
             final int index = this.index + ((y & 15) << 8);
-            return states[get.sections[newLayer].get(get, newLayer, index)];
+            return states[get.get(newLayer, index)];
         }
         return BlockTypes.__RESERVED__.getDefaultState();
     }

diff --git a/...ore/src/main/java/com/fastasyncworldedit/core/queue/implementation/blocks/CharBlocks.java b/...ore/src/main/java/com/fastasyncworldedit/core/queue/implementation/blocks/CharBlocks.java
@@ -9,7 +9,6 @@
 import org.apache.logging.log4j.Logger;
 
 import javax.annotation.Nullable;
-import java.lang.invoke.VarHandle;
 import java.util.Arrays;
 
 public abstract class CharBlocks implements IBlocks {
@@ -18,12 +17,11 @@ public abstract class CharBlocks implements IBlocks {
 
     protected static final Section FULL = new Section() {
         @Override
-        public char[] get(CharBlocks blocks, int layer) {
-            char[] arr = blocks.blocks[layer];
+        public char[] get(CharBlocks blocks, int layer, char[] arr) {
             if (arr == null) {
                 // Chunk probably trimmed mid-operations, but do nothing about it to avoid other issues
                 synchronized (blocks.sectionLocks[layer]) {
-                    LOGGER.warn("Unexpected null section, please report this occurence alongside a debugpaste.");
+                    LOGGER.warn("Unexpected null section, please report this occurrence alongside a debugpaste.");
                     return getSkipFull(blocks, layer, false);
                 }
             }
@@ -32,12 +30,11 @@ public char[] get(CharBlocks blocks, int layer) {
 
         // Ignore aggressive switch here.
         @Override
-        public char[] get(CharBlocks blocks, int layer, boolean aggressive) {
-            char[] arr = blocks.blocks[layer];
+        public char[] get(CharBlocks blocks, int layer, char[] arr, boolean aggressive) {
             if (arr == null) {
                 // Chunk probably trimmed mid-operations, but do nothing about it to avoid other issues
                 synchronized (blocks.sectionLocks[layer]) {
-                    LOGGER.warn("Unexpected null section, please report this occurence alongside a debugpaste.");
+                    LOGGER.warn("Unexpected null section, please report this occurrence alongside a debugpaste.");
                     return getSkipFull(blocks, layer, aggressive);
                 }
             }
@@ -51,17 +48,14 @@ public boolean isFull() {
     };
     protected static final Section EMPTY = new Section() {
         @Override
-        public char[] get(CharBlocks blocks, int layer) {
+        public char[] get(CharBlocks blocks, int layer, char[] arr) {
             // Defaults to aggressive as it should only be avoided where we know we've reset a chunk during an edit
-            return get(blocks, layer, true);
+            return get(blocks, layer, arr, true);
         }
 
         @Override
-        public char[] get(CharBlocks blocks, int layer, boolean aggressive) {
+        public char[] get(CharBlocks blocks, int layer, char[] arr, boolean aggressive) {
             synchronized (blocks.sectionLocks[layer]) {
-                if (blocks.sections[layer] == FULL) {
-                    return FULL.get(blocks, layer);
-                }
                 return getSkipFull(blocks, layer, aggressive);
             }
         }
@@ -72,7 +66,6 @@ public boolean isFull() {
         }
     };
     public char[][] blocks;
-    public Section[] sections;
     public Object[] sectionLocks;
     protected int minSectionPosition;
     protected int maxSectionPosition;
@@ -88,10 +81,8 @@ public CharBlocks(int minSectionPosition, int maxSectionPosition) {
         this.maxSectionPosition = maxSectionPosition;
         this.sectionCount = maxSectionPosition - minSectionPosition + 1;
         blocks = new char[sectionCount][];
-        sections = new Section[sectionCount];
         sectionLocks = new Object[sectionCount];
         for (int i = 0; i < sectionCount; i++) {
-            sections[i] = EMPTY;
             sectionLocks[i] = new Object();
         }
     }
@@ -102,45 +93,38 @@ public void init(int chunkX, int chunkZ) {
     }
 
     @Override
-    public synchronized boolean trim(boolean aggressive) {
-        boolean result = true;
+    public boolean trim(boolean aggressive) {
         for (int i = 0; i < sectionCount; i++) {
-            if (!sections[i].isFull() && blocks[i] != null) {
-                blocks[i] = null;
-            } else {
-                result = false;
+            synchronized (sectionLocks[i]) {
+                if (blocks[i] != null) {
+                    return false;
+                }
             }
         }
-        return result;
+        return true;
     }
 
     @Override
     public boolean trim(boolean aggressive, int layer) {
-        boolean result = true;
         synchronized (sectionLocks[layer]) {
-            if (!sections[layer].isFull() && blocks[layer] != null) {
-                blocks[layer] = null;
-            } else {
-                result = false;
-            }
+            return blocks[layer] == null;
         }
-        return result;
     }
 
     @Override
-    public synchronized IChunkSet reset() {
+    public IChunkSet reset() {
         for (int i = 0; i < sectionCount; i++) {
-            sections[i] = EMPTY;
-            VarHandle.storeStoreFence();
-            blocks[i] = null;
+            synchronized (sectionLocks[i]) {
+                blocks[i] = null;
+            }
         }
         return null;
     }
 
     public void reset(int layer) {
         layer -= minSectionPosition;
         synchronized (sectionLocks[layer]) {
-            sections[layer] = EMPTY;
+            blocks[layer] = null;
         }
     }
 
@@ -154,11 +138,10 @@ public char[] update(int layer, char[] data, boolean aggressive) {
 
     protected char[] loadPrivately(int layer) {
         layer -= getMinSectionPosition();
-        if (sections[layer] != null) {
-            synchronized (sectionLocks[layer]) {
-                if (sections[layer].isFull() && blocks[layer] != null) {
-                    return blocks[layer];
-                }
+        synchronized (sectionLocks[layer]) {
+            char[] data = blocks[layer];
+            if (data != null) {
+                return data;
             }
         }
         return update(layer, null, true);
@@ -168,15 +151,14 @@ protected char[] loadPrivately(int layer) {
     @Override
     public boolean hasSection(int layer) {
         layer -= minSectionPosition;
-        return layer >= 0 && layer < sections.length && sections[layer].isFull();
+        return layer >= 0 && layer < blocks.length && blocks[layer] != null;
     }
 
     @Override
     public char[] load(int layer) {
         layer -= minSectionPosition;
-        synchronized (sectionLocks[layer]) {
-            return sections[layer].get(this, layer);
-        }
+        char[] data = blocks[layer];
+        return (data == null ? EMPTY : FULL).get(this, layer, data);
     }
 
     @Nullable
@@ -186,7 +168,7 @@ public char[] loadIfPresent(int layer) {
             return null;
         }
         layer -= minSectionPosition;
-        return sections[layer].isFull() ? blocks[layer] : null;
+        return blocks[layer];
     }
 
     @Override
@@ -251,38 +233,17 @@ public void set(int x, int y, int z, char value) {
      */
 
     public final char get(int layer, int index) {
-        return sections[layer - minSectionPosition].get(this, layer, index);
+        char[] data = blocks[layer - minSectionPosition];
+        return (data == null ? EMPTY : FULL).get(this, layer, index, data);
     }
 
     public final void set(int layer, int index, char value) throws ArrayIndexOutOfBoundsException {
-        sections[layer - minSectionPosition].set(this, layer, index, value);
+        char[] data = blocks[layer - minSectionPosition];
+        (data == null ? EMPTY : FULL).set(this, layer, index, value, data);
     }
 
     public abstract static class Section {
 
-        abstract char[] get(CharBlocks blocks, int layer);
-
-        abstract char[] get(CharBlocks blocks, int layer, boolean aggressive);
-
-        public abstract boolean isFull();
-
-        public final char get(CharBlocks blocks, int layer, int index) {
-            int normalized = layer - blocks.minSectionPosition;
-            char[] section = get(blocks, normalized);
-            if (section == null) {
-                synchronized (blocks.sectionLocks[normalized]) {
-                    blocks.reset(layer);
-                    section = EMPTY.get(blocks, normalized, false);
-                }
-            }
-            return section[index];
-        }
-
-        public final synchronized void set(CharBlocks blocks, int layer, int index, char value) {
-            layer -= blocks.minSectionPosition;
-            get(blocks, layer)[index] = value;
-        }
-
         static char[] getSkipFull(CharBlocks blocks, int layer, boolean aggressive) {
             char[] arr = blocks.blocks[layer];
             if (arr == null) {
@@ -296,12 +257,32 @@ static char[] getSkipFull(CharBlocks blocks, int layer, boolean aggressive) {
                     throw new IllegalStateException("Array cannot be null (update): " + blocks.getClass());
                 }
             }
-            if (blocks.blocks[layer] != null) {
-                blocks.sections[layer] = FULL;
-            }
             return arr;
         }
 
+        abstract char[] get(CharBlocks blocks, int layer, char[] data);
+
+        abstract char[] get(CharBlocks blocks, int layer, char[] data, boolean aggressive);
+
+        public abstract boolean isFull();
+
+        public final char get(CharBlocks blocks, int layer, int index, char[] data) {
+            int normalized = layer - blocks.minSectionPosition;
+            char[] section = get(blocks, normalized, data);
+            if (section == null) {
+                synchronized (blocks.sectionLocks[normalized]) {
+                    blocks.reset(layer);
+                    section = EMPTY.get(blocks, normalized, data, false);
+                }
+            }
+            return section[index];
+        }
+
+        public final synchronized void set(CharBlocks blocks, int layer, int index, char value, char[] data) {
+            layer -= blocks.minSectionPosition;
+            get(blocks, layer, data)[index] = value;
+        }
+
     }
 
 }
diff --git a/.../src/main/java/com/fastasyncworldedit/core/queue/implementation/blocks/CharGetBlocks.java b/.../src/main/java/com/fastasyncworldedit/core/queue/implementation/blocks/CharGetBlocks.java
@@ -25,10 +25,11 @@ public BaseBlock getFullBlock(int x, int y, int z) {
     }
 
     @Override
-    public synchronized boolean trim(boolean aggressive) {
+    public boolean trim(boolean aggressive) {
         for (int i = 0; i < sectionCount; i++) {
-            sections[i] = EMPTY;
-            blocks[i] = null;
+            synchronized (sectionLocks[i]) {
+                blocks[i] = null;
+            }
         }
         return true;
     }
@@ -48,11 +49,12 @@ protected char defaultOrdinal() {
     }
 
     @Override
-    public synchronized boolean trim(boolean aggressive, int layer) {
+    public boolean trim(boolean aggressive, int layer) {
         layer -= minSectionPosition;
-        sections[layer] = EMPTY;
-        blocks[layer] = null;
-        return true;
+        synchronized (sectionLocks[layer]) {
+            blocks[layer] = null;
+            return true;
+        }
     }
 
     @Override