From 45400a968562a6f6d880f7c2de97cfcc1f1ab8f6 Mon Sep 17 00:00:00 2001
From: Babbaj <babbaj45@gmail.com>
Date: Mon, 24 Jul 2023 03:30:06 -0400
Subject: [PATCH] fix concurrency concerns

---
 .../elytra/BlockStateOctreeInterface.java       | 11 ++++-------
 .../baritone/process/elytra/ElytraBehavior.java | 17 +++++++++++------
 .../process/elytra/NetherPathfinderContext.java |  7 ++++---
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/main/java/baritone/process/elytra/BlockStateOctreeInterface.java b/src/main/java/baritone/process/elytra/BlockStateOctreeInterface.java
index f9e700085..efbfb9216 100644
--- a/src/main/java/baritone/process/elytra/BlockStateOctreeInterface.java
+++ b/src/main/java/baritone/process/elytra/BlockStateOctreeInterface.java
@@ -27,7 +27,7 @@ public final class BlockStateOctreeInterface {
 
     private final NetherPathfinderContext context;
     private final long contextPtr;
-    volatile long chunkPtr;
+    transient long chunkPtr;
 
     // Guarantee that the first lookup will fetch the context by setting MAX_VALUE
     private int prevChunkX = Integer.MAX_VALUE;
@@ -44,14 +44,11 @@ public final class BlockStateOctreeInterface {
         }
         final int chunkX = x >> 4;
         final int chunkZ = z >> 4;
-        long pointer = this.chunkPtr;
-        if (pointer == 0 | ((chunkX ^ this.prevChunkX) | (chunkZ ^ this.prevChunkZ)) != 0) {
+        if (this.chunkPtr == 0 | ((chunkX ^ this.prevChunkX) | (chunkZ ^ this.prevChunkZ)) != 0) {
             this.prevChunkX = chunkX;
             this.prevChunkZ = chunkZ;
-            synchronized (this.context.cacheLock) {
-                this.chunkPtr = pointer = NetherPathfinder.getOrCreateChunk(this.contextPtr, chunkX, chunkZ);
-            }
+            this.chunkPtr = NetherPathfinder.getOrCreateChunk(this.contextPtr, chunkX, chunkZ);
         }
-        return Octree.getBlock(pointer, x & 0xF, y & 0x7F, z & 0xF);
+        return Octree.getBlock(this.chunkPtr, x & 0xF, y & 0x7F, z & 0xF);
     }
 }
diff --git a/src/main/java/baritone/process/elytra/ElytraBehavior.java b/src/main/java/baritone/process/elytra/ElytraBehavior.java
index a99b4e331..0e6b7ad2a 100644
--- a/src/main/java/baritone/process/elytra/ElytraBehavior.java
+++ b/src/main/java/baritone/process/elytra/ElytraBehavior.java
@@ -446,6 +446,17 @@ public final class ElytraBehavior implements Helper {
     }
 
     public void onTick() {
+        synchronized (this.context.cullingLock) {
+            this.onTick0();
+        }
+        final long now = System.currentTimeMillis();
+        if ((now - this.timeLastCacheCull) / 1000 > Baritone.settings().elytraTimeBetweenCacheCullSecs.value) {
+            this.context.queueCacheCulling(ctx.player().chunkCoordX, ctx.player().chunkCoordZ, Baritone.settings().elytraCacheCullDistance.value, this.boi);
+            this.timeLastCacheCull = now;
+        }
+    }
+
+    private void onTick0() {
         // Fetch the previous solution, regardless of if it's going to be used
         this.pendingSolution = null;
         if (this.solver != null) {
@@ -498,12 +509,6 @@ public final class ElytraBehavior implements Helper {
                 Math.max(playerNear - 30, 0),
                 Math.min(playerNear + 100, path.size())
         );
-
-        final long now = System.currentTimeMillis();
-        if ((now - this.timeLastCacheCull) / 1000 > Baritone.settings().elytraTimeBetweenCacheCullSecs.value) {
-            this.context.queueCacheCulling(ctx.player().chunkCoordX, ctx.player().chunkCoordZ, Baritone.settings().elytraCacheCullDistance.value, this.boi);
-            this.timeLastCacheCull = now;
-        }
     }
 
     /**
diff --git a/src/main/java/baritone/process/elytra/NetherPathfinderContext.java b/src/main/java/baritone/process/elytra/NetherPathfinderContext.java
index 69ac41992..991457d10 100644
--- a/src/main/java/baritone/process/elytra/NetherPathfinderContext.java
+++ b/src/main/java/baritone/process/elytra/NetherPathfinderContext.java
@@ -38,7 +38,6 @@ import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * @author Brady
@@ -46,7 +45,9 @@ import java.util.concurrent.atomic.AtomicInteger;
 public final class NetherPathfinderContext {
 
     private static final IBlockState AIR_BLOCK_STATE = Blocks.AIR.getDefaultState();
-    public final Object cacheLock = new Object();
+    // This lock must be held while there are active pointers to chunks in java,
+    // but we just hold it for the entire tick so we don't have to think much about it.
+    public final Object cullingLock = new Object();
 
     // Visible for access in BlockStateOctreeInterface
     final long context;
@@ -61,7 +62,7 @@ public final class NetherPathfinderContext {
 
     public void queueCacheCulling(int chunkX, int chunkZ, int maxDistanceBlocks, BlockStateOctreeInterface boi) {
         this.executor.execute(() -> {
-            synchronized (this.cacheLock) {
+            synchronized (this.cullingLock) {
                 boi.chunkPtr = 0L;
                 NetherPathfinder.cullFarChunks(this.context, chunkX, chunkZ, maxDistanceBlocks);
             }