fix some data sources not being caught and closed

This should further reduce the max memory needed.

DelayedFullDataSourceSaveCache is problematic due to not properly closing all data sources it creates and has been deprecated and removed.

core/src/main/java/com/seibel/distanthorizons/core/dataObjects/transformers/FullDataToRenderDataTransformer.java

@@ -164,7 +164,6 @@ public class FullDataToRenderDataTransformer
 		}
 		else
 		{
-			// new LongArrayList(new long[fullDataLength])
 			PhantomArrayListCheckout checkout = ARRAY_LIST_POOL.checkoutArrays(0, 0, 1);
 			LongArrayList dataArrayList = checkout.getLongArray(0, fullDataLength);
 			

core/src/main/java/com/seibel/distanthorizons/core/file/AbstractDataSourceHandler.java

@@ -69,8 +69,6 @@ public abstract class AbstractDataSourceHandler
 	
 	public final ArrayList<IDataSourceUpdateFunc<TDataSource>> dateSourceUpdateListeners = new ArrayList<>();
 	
-	public final ConcurrentHashMap<Long, CompletableFuture<Void>> updateDataSourceFutureByPos = new ConcurrentHashMap<>();
-	
 	
 	
 	//=============//
@@ -186,6 +184,17 @@ public abstract class AbstractDataSourceHandler
 	// data updating //
 	//===============//
 	
+	/** 
+	 * Can be used if the same thread is already handling IO and/or LOD generation.
+	 * Otherwise the async version {@link AbstractDataSourceHandler#updateDataSourceAsync(FullDataSourceV2)} may be a better choice.
+	 */
+	public void updateDataSource(@NotNull FullDataSourceV2 inputDataSource)
+	{ this.updateDataSourceAtPos(inputDataSource.getPos(), inputDataSource, true); }
+	
+	/**
+	 * Can be used if you don't want to lock the current thread
+	 * Otherwise the sync version {@link AbstractDataSourceHandler#updateDataSource(FullDataSourceV2)} may be a better choice.
+	 */
 	public CompletableFuture<Void> updateDataSourceAsync(@NotNull FullDataSourceV2 inputDataSource)
 	{
 		AbstractExecutorService executor = ThreadPoolUtil.getChunkToLodBuilderExecutor();
@@ -197,35 +206,22 @@ public abstract class AbstractDataSourceHandler
 		
 		try
 		{
-			return this.updateDataSourceFutureByPos.compute(inputDataSource.getPos(), (Long newPos, CompletableFuture<Void> future) -> 
+			this.markUpdateStart(inputDataSource.getPos());
+			return CompletableFuture.runAsync(() ->
 			{
-				if (future != null)
+				try
 				{
-					future.cancel(false);
-					this.markUpdateEnd(newPos);
+					this.updateDataSourceAtPos(inputDataSource.getPos(), inputDataSource, true);
 				}
-				
-				// run file handling on a separate thread
-				this.markUpdateStart(newPos);
-				future = CompletableFuture.runAsync(() ->
+				catch (Exception e)
 				{
-					try
-					{
-						this.updateDataSourceAtPos(newPos, inputDataSource, true);
-					}
-					catch (Exception e)
-					{
-						LOGGER.error("Unexpected error in async data source update at pos: ["+DhSectionPos.toString(newPos)+"], error: ["+e.getMessage()+"].", e);
-					}
-					finally
-					{
-						this.markUpdateEnd(newPos);
-						this.updateDataSourceFutureByPos.remove(newPos);
-					}
-				}, executor);
-				
-				return future;
-			});
+					LOGGER.error("Unexpected error in async data source update at pos: ["+DhSectionPos.toString(inputDataSource.getPos())+"], error: ["+e.getMessage()+"].", e);
+				}
+				finally
+				{
+					this.markUpdateEnd(inputDataSource.getPos());
+				}
+			}, executor);
 		}
 		catch (RejectedExecutionException ignore)
 		{

core/src/main/java/com/seibel/distanthorizons/core/file/fullDatafile/DelayedFullDataSourceSaveCache.java

@@ -10,11 +10,16 @@ import java.util.Timer;
 import java.util.TimerTask;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * Used to batch together multiple data source updates that all
  * affect the same position.
+ * 
+ * @deprecated due to causing data source leaks, however we may still want to re-visit this 
+ * if saving directly is too slow for certain operations (specifically modifying nearby chunks).
  */
+@Deprecated
 public class DelayedFullDataSourceSaveCache
 {
 	private static final Logger LOGGER = DhLoggerBuilder.getLogger();
@@ -26,7 +31,12 @@ public class DelayedFullDataSourceSaveCache
 	private final ConcurrentHashMap<Long, TimerTask> saveTimerTasksBySectionPos = new ConcurrentHashMap<>();
 	private final ConcurrentHashMap<Long, CompletableFuture<Void>> futureBySectionPos = new ConcurrentHashMap<>();
 	
-	private final ISaveDataSourceFunc onSaveTimeoutFunc;
+	protected final ReentrantLock[] saveLockArray;
+	/** Based on the stack overflow post: https://stackoverflow.com/a/45909920 */
+	protected ReentrantLock getSaveLockForPos(long pos) { return this.saveLockArray[Math.abs(Long.hashCode(pos)) % this.saveLockArray.length]; }
+	
+	
+	private final ISaveDataSourceFunc onSaveTimeoutAsyncFunc;
 	private final int saveDelayInMs;
 	
 	
@@ -35,10 +45,20 @@ public class DelayedFullDataSourceSaveCache
 	// constructor //
 	//=============//
 	
-	public DelayedFullDataSourceSaveCache(@NotNull ISaveDataSourceFunc onSaveTimeoutFunc, int saveDelayInMs)
+	public DelayedFullDataSourceSaveCache(@NotNull ISaveDataSourceFunc onSaveTimeoutAsyncFunc, int saveDelayInMs)
 	{
-		this.onSaveTimeoutFunc = onSaveTimeoutFunc;
+		this.onSaveTimeoutAsyncFunc = onSaveTimeoutAsyncFunc;
 		this.saveDelayInMs = saveDelayInMs;
+		
+		
+		// the lock array's length is 2x the number of CPU cores so the number of collisions
+		// should be relatively low without having too many extra locks
+		int lockCount = Runtime.getRuntime().availableProcessors() * 2;
+		this.saveLockArray = new ReentrantLock[lockCount];
+		for (int i = 0; i < lockCount; i++)
+		{
+			this.saveLockArray[i] = new ReentrantLock();
+		}
 	}
 	
 	
@@ -53,73 +73,90 @@ public class DelayedFullDataSourceSaveCache
 	 */
 	public CompletableFuture<Void> writeDataSourceToMemoryAndQueueSave(FullDataSourceV2 inputDataSource)
 	{
-		long dataSourcePos = inputDataSource.getPos();
 		
-		CompletableFuture<Void> future = this.futureBySectionPos.computeIfAbsent(dataSourcePos, (inputPos) -> new CompletableFuture<>());
-		
-		this.dataSourceByPosition.compute(dataSourcePos, (inputPos, memoryDataSource) ->
+		boolean saveNow = true;
+		if (saveNow)
 		{
-			if (memoryDataSource == null)
-			{
-				// should not be closed since it will be used by other threads 
-				memoryDataSource = FullDataSourceV2.createEmpty(inputPos);
-			}
+			// TODO this doesn't leak, but also doesn't delay the save any
+			FullDataSourceV2 memoryDataSource = FullDataSourceV2.createEmpty(inputDataSource.getPos());
 			memoryDataSource.update(inputDataSource);
-			
-			
-			TimerTask timerTask = new TimerTask()
-			{
-				@Override
-				public void run()
+			return this.onSaveTimeoutAsyncFunc.saveAsync(memoryDataSource)
+				.handle((voidObj, exception) -> 
 				{
-					DelayedFullDataSourceSaveCache.this.saveTimerTasksBySectionPos.remove(dataSourcePos);
-					
-					try
-					{
-						FullDataSourceV2 dataSourceToSave = DelayedFullDataSourceSaveCache.this.dataSourceByPosition.remove(dataSourcePos);
-						if (dataSourceToSave != null)
-						{
-							DelayedFullDataSourceSaveCache.this.onSaveTimeoutFunc.save(dataSourceToSave);
-						}
-					}
-					catch (Exception e) // this can throw errors (not exceptions) when installed in Iris' dev environment for some reason due to an issue with LZ4's compression library
-					{
-						LOGGER.error("Failed to save updated data for section ["+dataSourcePos+"], error: ["+e.getMessage()+"]", e);
-					}
-					finally
-					{
-						CompletableFuture<Void> future = DelayedFullDataSourceSaveCache.this.futureBySectionPos.remove(dataSourcePos);
-						if (future != null)
-						{
-							future.complete(null);
-						}
-					}
+					memoryDataSource.close();
+					return null;
+				});
+		}
+		else
+		{
+			long dataSourcePos = inputDataSource.getPos();
+			
+			CompletableFuture<Void> future = this.futureBySectionPos.computeIfAbsent(dataSourcePos, (inputPos) -> new CompletableFuture<>());
+			
+			this.dataSourceByPosition.compute(dataSourcePos, (inputPos, memoryDataSource) ->
+			{
+				if (memoryDataSource == null)
+				{
+					// should not be closed since it will be used by other threads 
+					memoryDataSource = FullDataSourceV2.createEmpty(inputPos);
 				}
-			};
-			try
-			{
-				DELAY_UPDATE_TIMER.schedule(timerTask, this.saveDelayInMs);
-			}
-			catch (IllegalStateException ignore)
-			{
-				// James isn't sure why this is possible since this logic is inside a lock, 
-				// maybe the timer is just async enough that there can be problems?
-				//LOGGER.warn("Attempted to queue an already canceled task. Pos: ["+dataSourcePos+"], task already queued for pos: ["+this.saveTimerTasksBySectionPos.containsKey(dataSourcePos)+"]");
-			}
+				memoryDataSource.update(inputDataSource);
+				
+				
+				TimerTask timerTask = new TimerTask()
+				{
+					@Override
+					public void run()
+					{
+						DelayedFullDataSourceSaveCache.this.saveTimerTasksBySectionPos.remove(dataSourcePos);
+						
+						try
+						{
+							FullDataSourceV2 dataSourceToSave = DelayedFullDataSourceSaveCache.this.dataSourceByPosition.remove(dataSourcePos);
+							if (dataSourceToSave != null)
+							{
+								DelayedFullDataSourceSaveCache.this.onSaveTimeoutAsyncFunc.saveAsync(dataSourceToSave);
+							}
+						}
+						catch (Exception e) // this can throw errors (not exceptions) when installed in Iris' dev environment for some reason due to an issue with LZ4's compression library
+						{
+							LOGGER.error("Failed to save updated data for section ["+dataSourcePos+"], error: ["+e.getMessage()+"]", e);
+						}
+						finally
+						{
+							CompletableFuture<Void> future = DelayedFullDataSourceSaveCache.this.futureBySectionPos.remove(dataSourcePos);
+							if (future != null)
+							{
+								future.complete(null);
+							}
+						}
+					}
+				};
+				try
+				{
+					DELAY_UPDATE_TIMER.schedule(timerTask, this.saveDelayInMs);
+				}
+				catch (IllegalStateException ignore)
+				{
+					// James isn't sure why this is possible since this logic is inside a lock, 
+					// maybe the timer is just async enough that there can be problems?
+					//LOGGER.warn("Attempted to queue an already canceled task. Pos: ["+dataSourcePos+"], task already queued for pos: ["+this.saveTimerTasksBySectionPos.containsKey(dataSourcePos)+"]");
+				}
+				
+				
+				// cancel the old save timer if present
+				// (this is equivalent to restarting the timer)
+				TimerTask oldTask = this.saveTimerTasksBySectionPos.put(dataSourcePos, timerTask);
+				if (oldTask != null)
+				{
+					oldTask.cancel();
+				}
+				
+				return memoryDataSource;
+			});
 			
-			
-			// cancel the old save timer if present
-			// (this is equivalent to restarting the timer)
-			TimerTask oldTask = this.saveTimerTasksBySectionPos.put(dataSourcePos, timerTask);
-			if (oldTask != null)
-			{
-				oldTask.cancel();
-			}
-			
-			return memoryDataSource;
-		});
-		
-		return future;
+			return future;
+		}
 	}
 	
 	public int getUnsavedCount() { return this.dataSourceByPosition.size(); }
@@ -142,7 +179,7 @@ public class DelayedFullDataSourceSaveCache
 	public interface ISaveDataSourceFunc
 	{
 		/** called after the timeout expires */
-		void save(FullDataSourceV2 inputDataSource);
+		CompletableFuture<Void> saveAsync(FullDataSourceV2 inputDataSource);
 	}
 	
 }

core/src/main/java/com/seibel/distanthorizons/core/file/fullDatafile/FullDataSourceProviderV2.java

@@ -453,14 +453,7 @@ public class FullDataSourceProviderV2
 								{
 									// after the update finishes the legacy data source can be safely deleted
 									this.legacyFileHandler.repo.deleteWithKey(legacyDataSource.getPos());
-									
-									try
-									{
-										newDataSource.close();
-									}
-									catch (Exception ignore)
-									{
-									}
+									newDataSource.close();
 								});
 							}
 							catch (Exception e)
@@ -626,13 +619,6 @@ public class FullDataSourceProviderV2
 	/** Can be used to display how many total chunk retrieval requests should be available. */
 	public void setEstimatedRemainingRetrievalChunkCount(int newCount) { }
 	
-	/** 
-	 * Returns how many data sources are currently in memory and haven't
-	 * been saved to the database.
-	 * Returns -1 if this provider never stores data sources to memory.
-	 */
-	public int getUnsavedDataSourceCount() { return -1; }
-	
 	public boolean fileExists(long pos) { return this.repo.getDataSizeInBytes(pos) > 0; }
 	
 	

core/src/main/java/com/seibel/distanthorizons/core/file/fullDatafile/GeneratedFullDataSourceProvider.java

@@ -44,7 +44,6 @@ import org.apache.logging.log4j.Logger;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.awt.*;
 import java.io.File;
 import java.util.*;
 import java.util.concurrent.*;
@@ -71,8 +70,6 @@ public class GeneratedFullDataSourceProvider extends FullDataSourceProviderV2 im
 	private final AtomicReference<IFullDataSourceRetrievalQueue> worldGenQueueRef = new AtomicReference<>(null);
 	private final ArrayList<IOnWorldGenCompleteListener> onWorldGenTaskCompleteListeners = new ArrayList<>();
 	
-	protected final DelayedFullDataSourceSaveCache delayedFullDataSourceSaveCache = new DelayedFullDataSourceSaveCache(this::onDataSourceSave, 5_000);
-	
 	
 	
 	//=============//
@@ -214,17 +211,6 @@ public class GeneratedFullDataSourceProvider extends FullDataSourceProviderV2 im
 		
 		int maxQueueCount = MAX_WORLD_GEN_REQUESTS_PER_THREAD * Config.Common.MultiThreading.numberOfThreads.get();
 		
-		if (this.delayedFullDataSourceSaveCache.getUnsavedCount() >= maxQueueCount)
-		{
-			// flushing since we're waiting for this timer to expire anyway
-			this.delayedFullDataSourceSaveCache.flush();
-			
-			// don't queue additional world gen requests if there are
-			// a lot of data sources in memory 
-			// (this is done to prevent infinite memory growth)
-			return false;
-		}
-		
 		int availableTaskSlots = maxQueueCount - worldGenQueue.getWaitingTaskCount();
 		if (availableTaskSlots <= 0)
 		{
@@ -287,9 +273,6 @@ public class GeneratedFullDataSourceProvider extends FullDataSourceProviderV2 im
 	@Override
 	public void clearRetrievalQueue() { this.worldGenQueueRef.set(null); }
 	
-	@Override
-	public int getUnsavedDataSourceCount() { return this.delayedFullDataSourceSaveCache.getUnsavedCount(); }
-	
 	
 	public boolean isFullyGenerated(ByteArrayList columnGenerationSteps)
 	{
@@ -413,22 +396,6 @@ public class GeneratedFullDataSourceProvider extends FullDataSourceProviderV2 im
 	
 	
 	
-	//=======//
-	// debug //
-	//=======//
-	
-	@Override
-	public void debugRender(DebugRenderer renderer)
-	{
-		super.debugRender(renderer);
-		
-		this.delayedFullDataSourceSaveCache.dataSourceByPosition
-				.forEach((pos, dataSource) -> { renderer.renderBox(new DebugRenderer.Box(pos, -32f, 80f, 0.20f, Color.green.darker())); });
-	}
-	
-	
-	
-	
 	//================//
 	// helper classes //
 	//================//
@@ -447,7 +414,7 @@ public class GeneratedFullDataSourceProvider extends FullDataSourceProviderV2 im
 		{
 			return (dataSource) ->
 			{
-				GeneratedFullDataSourceProvider.this.delayedFullDataSourceSaveCache.writeDataSourceToMemoryAndQueueSave(dataSource);
+				GeneratedFullDataSourceProvider.this.onDataSourceSave(dataSource);
 			};
 		}
 		
@@ -459,7 +426,7 @@ public class GeneratedFullDataSourceProvider extends FullDataSourceProviderV2 im
 		// allows us to reduce cross-chunk lighting issues by lighting the whole 4x4 LOD at once
 		DhLightingEngine.INSTANCE.bakeDataSourceSkyLight(fullDataSource, LodUtil.MAX_MC_LIGHT);
 		
-		GeneratedFullDataSourceProvider.this.updateDataSourceAsync(fullDataSource); 
+		this.updateDataSource(fullDataSource);
 	}
 	
 	

core/src/main/java/com/seibel/distanthorizons/core/file/fullDatafile/RemoteFullDataSourceProvider.java

@@ -99,7 +99,7 @@ public class RemoteFullDataSourceProvider extends GeneratedFullDataSourceProvide
 		Long timestamp = this.getTimestampForPos(pos);
 		if (timestamp != null)
 		{
-			this.syncOnLoadRequestQueue.submitRequest(pos, timestamp, this.delayedFullDataSourceSaveCache::writeDataSourceToMemoryAndQueueSave);
+			this.syncOnLoadRequestQueue.submitRequest(pos, timestamp, this::updateDataSource);
 		}
 		
 		return super.get(pos);

core/src/main/java/com/seibel/distanthorizons/core/level/AbstractDhLevel.java

@@ -59,8 +59,7 @@ public abstract class AbstractDhLevel implements IDhLevel
 	@Nullable
 	public BeaconBeamRepo beaconBeamRepo;
 	
-	protected final DelayedFullDataSourceSaveCache delayedFullDataSourceSaveCache = new DelayedFullDataSourceSaveCache(this::onDataSourceSave, 500);
-	/** contains the {@link DhChunkPos} for each {@link DhSectionPos} that are queued to save via {@link AbstractDhLevel#delayedFullDataSourceSaveCache} */
+	/** contains the {@link DhChunkPos} for each {@link DhSectionPos} that are queued to save */
 	protected final ConcurrentHashMap<Long, HashSet<DhChunkPos>> updatedChunkPosSetBySectionPos = new ConcurrentHashMap<>();
 	protected final ConcurrentHashMap<DhChunkPos, Integer> updatedChunkHashesByChunkPos = new ConcurrentHashMap<>();
 	
@@ -141,39 +140,37 @@ public abstract class AbstractDhLevel implements IDhLevel
 	// default methods //
 	//=================//
 	
-	@Override
-	public int getUnsavedDataSourceCount() { return this.delayedFullDataSourceSaveCache.getUnsavedCount(); }
-	
 	@Override
 	public CompletableFuture<Void> updateChunkAsync(IChunkWrapper chunkWrapper, int chunkHash)
 	{
-		// data source synchronously written to memory so it can be safely closed
-		try (FullDataSourceV2 dataSource = FullDataSourceV2.createFromChunk(chunkWrapper))
+		FullDataSourceV2 dataSource = FullDataSourceV2.createFromChunk(chunkWrapper);
+		if (dataSource == null)
 		{
-			if (dataSource == null)
-			{
-				// This can happen if, among other reasons, a chunk save is superseded by a later event
-				return CompletableFuture.completedFuture(null);
-			}
-			
-			
-			this.updatedChunkPosSetBySectionPos.compute(dataSource.getPos(), (dataSourcePos, chunkPosSet) ->
-			{
-				if (chunkPosSet == null)
-				{
-					chunkPosSet = new HashSet<>();
-				}
-				chunkPosSet.add(chunkWrapper.getChunkPos());
-				return chunkPosSet;
-			});
-			this.updatedChunkHashesByChunkPos.put(chunkWrapper.getChunkPos(), chunkHash);
-			
-			// batch updates to reduce overhead when flying around or breaking/placing a lot of blocks in an area
-			return this.delayedFullDataSourceSaveCache.writeDataSourceToMemoryAndQueueSave(dataSource);
+			// This can happen if, among other reasons, a chunk save is superseded by a later event
+			return CompletableFuture.completedFuture(null);
 		}
+		
+		
+		this.updatedChunkPosSetBySectionPos.compute(dataSource.getPos(), (dataSourcePos, chunkPosSet) ->
+		{
+			if (chunkPosSet == null)
+			{
+				chunkPosSet = new HashSet<>();
+			}
+			chunkPosSet.add(chunkWrapper.getChunkPos());
+			return chunkPosSet;
+		});
+		this.updatedChunkHashesByChunkPos.put(chunkWrapper.getChunkPos(), chunkHash);
+		
+		return this.onDataSourceSaveAsync(dataSource)
+			.handle((voidObj, throwable) -> 
+			{
+				dataSource.close();
+				return null;
+			});
 	}
 	
-	private void onDataSourceSave(FullDataSourceV2 fullDataSource)
+	private CompletableFuture<Void> onDataSourceSaveAsync(FullDataSourceV2 fullDataSource)
 	{
 		// block lights should have been populated at the chunkWrapper stage
 		// waiting to populate the data source's skylight at this stage prevents re-lighting and
@@ -181,7 +178,8 @@ public abstract class AbstractDhLevel implements IDhLevel
 		DhLightingEngine.INSTANCE.bakeDataSourceSkyLight(fullDataSource, this.hasSkyLight() ? LodUtil.MAX_MC_LIGHT : LodUtil.MIN_MC_LIGHT);
 		
 		
-		this.updateDataSourcesAsync(fullDataSource).thenRun(() -> 
+		return this.updateDataSourcesAsync(fullDataSource)
+				.thenRun(() -> 
 		{
 			HashSet<DhChunkPos> updatedChunkPosSet = this.updatedChunkPosSetBySectionPos.remove(fullDataSource.getPos());
 			if (updatedChunkPosSet != null)

core/src/main/java/com/seibel/distanthorizons/core/level/AbstractDhServerLevel.java

@@ -277,7 +277,7 @@ public abstract class AbstractDhServerLevel extends AbstractDhLevel implements I
 						}
 					}
 				}
-				});
+			});
 	}
 	
 	

core/src/main/java/com/seibel/distanthorizons/core/level/IDhLevel.java

@@ -62,12 +62,6 @@ public interface IDhLevel extends AutoCloseable, GeneratedFullDataSourceProvider
 	
 	CompletableFuture<Void> updateDataSourcesAsync(FullDataSourceV2 data);
 	
-	/** 
-	 * this number is generally related to how many data sources have been updated 
-	 * due to chunk modifications or loads.
-	 */
-	int getUnsavedDataSourceCount();
-	
 	void addDebugMenuStringsToList(List<String> messageList);
 	
 	/** 

core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListCheckout.java

@@ -1,9 +1,11 @@
 package com.seibel.distanthorizons.core.pooling;
 
 import com.seibel.distanthorizons.core.util.ListUtil;
+import com.seibel.distanthorizons.coreapi.util.StringUtil;
 import it.unimi.dsi.fastutil.bytes.ByteArrayList;
 import it.unimi.dsi.fastutil.longs.LongArrayList;
 import it.unimi.dsi.fastutil.shorts.ShortArrayList;
+import org.jetbrains.annotations.Nullable;
 
 import java.lang.ref.SoftReference;
 import java.util.ArrayList;
@@ -19,6 +21,9 @@ public class PhantomArrayListCheckout implements AutoCloseable
 {
 	/** defines which pool the arrays should be returned too */
 	private final PhantomArrayListPool owningPool;
+	/** Will be null if the parent pool doesn't want leak stack tracing */
+	@Nullable
+	public final String allocationStackTrace;
 	
 	private final ArrayList<ByteArrayList> byteArrayLists = new ArrayList<>();
 	private final ArrayList<ShortArrayList> shortArrayLists = new ArrayList<>();
@@ -33,6 +38,17 @@ public class PhantomArrayListCheckout implements AutoCloseable
 	
 	public PhantomArrayListCheckout(PhantomArrayListPool owningPool)
 	{
+		if (owningPool.logGarbageCollectedStacks)
+		{
+			// TODO remove the top 4 or so lines since those will always be the same (relating to the phantom allocations)
+			//  and aren't helpful when debugging
+			this.allocationStackTrace = StringUtil.join("\n", Thread.currentThread().getStackTrace());
+		}
+		else
+		{
+			this.allocationStackTrace = null;
+		}
+		
 		this.owningPool = owningPool;
 	}
 	

core/src/main/java/com/seibel/distanthorizons/core/pooling/PhantomArrayListPool.java

@@ -6,6 +6,8 @@ import com.seibel.distanthorizons.core.logging.DhLoggerBuilder;
 import com.seibel.distanthorizons.core.logging.f3.F3Screen;
 import com.seibel.distanthorizons.core.util.LodUtil;
 import com.seibel.distanthorizons.core.util.ThreadUtil;
+import com.seibel.distanthorizons.core.util.objects.Pair;
+import com.seibel.distanthorizons.coreapi.ModInfo;
 import com.seibel.distanthorizons.coreapi.util.StringUtil;
 import it.unimi.dsi.fastutil.bytes.ByteArrayList;
 import it.unimi.dsi.fastutil.longs.LongArrayList;
@@ -50,15 +52,17 @@ public class PhantomArrayListPool
 	private static final Logger LOGGER = DhLoggerBuilder.getLogger();
 	
 	/** 
-	 * the recycler thread needs to be triggered relatively often to prevent
+	 * the recycler thread needs to be triggered relatively frequently to prevent
 	 * build up of GC'ed arrays.
+	 * However, some JVM's will wait a while before collecting lost objects,
+	 * so in general it is still much better to use the try-finally.
 	 */
-	private static final int PHANTOM_REF_CHECK_TIME_IN_MS = 1_000;
+	private static final int PHANTOM_REF_CHECK_TIME_IN_MS = 5_000;
 	private static final ThreadPoolExecutor RECYCLER_THREAD = ThreadUtil.makeSingleDaemonThreadPool("Phantom Array Recycler");
 	private static final ArrayList<PhantomArrayListPool> POOL_LIST = new ArrayList<>();
 	
 	/** if enabled the number of GC'ed arrays will be logged */
-	private static final boolean LOG_ARRAY_RECOVERY = false;
+	private static final boolean LOG_ARRAY_RECOVERY = ModInfo.IS_DEV_BUILD;
 	
 	
 	private static boolean lowMemoryWarningLogged = false; 
@@ -67,6 +71,12 @@ public class PhantomArrayListPool
 	
 	/** used for debugging and tracking what the pool contains */
 	public final String name;
+	/** 
+	 * Getting stack traces is very slow.
+	 * If we know which pool is leaking objects we can enable tracking for that specific
+	 * pool and prevent slow-downs in other pools.
+	 */
+	public final boolean logGarbageCollectedStacks;
 	
 	public final ConcurrentHashMap<Reference<? extends PhantomArrayListParent>, PhantomArrayListCheckout>
 			phantomRefToCheckout = new ConcurrentHashMap<>();
@@ -74,7 +84,6 @@ public class PhantomArrayListPool
 	
 	
 	
-	
 	private final ConcurrentLinkedQueue<ByteArrayList> pooledByteArrays = new ConcurrentLinkedQueue<>();
 	private final ConcurrentLinkedQueue<ShortArrayList> pooledShortArrays = new ConcurrentLinkedQueue<>();
 	private final ConcurrentLinkedQueue<SoftReference<LongArrayList>> pooledLongArrays = new ConcurrentLinkedQueue<>();
@@ -98,89 +107,27 @@ public class PhantomArrayListPool
 	
 	
 	
-	//=============//
-	// constructor //
-	//=============//
+	//==============//
+	// constructors //
+	//==============//
 	
 	// shared setup used by all pools
 	static
 	{
 		RECYCLER_THREAD.execute(() -> runPhantomReferenceCleanupLoop());
 	}
-	private static void runPhantomReferenceCleanupLoop()
-	{
-		while (true)
-		{
-			try
-			{
-				try
-				{
-					Thread.sleep(PHANTOM_REF_CHECK_TIME_IN_MS);
-				}
-				catch (InterruptedException ignore) { }
-				
-				
-				for (int i = 0; i < POOL_LIST.size(); i++)
-				{
-					PhantomArrayListPool pool = POOL_LIST.get(i);
-					
-					int returnedByteArrayCount = 0;
-					int returnedShortArrayCount = 0;
-					int returnedLongArrayCount = 0;
-					Reference<? extends PhantomArrayListParent> phantomRef = pool.phantomRefQueue.poll();
-					while (phantomRef != null)
-					{
-						// return the pooled arrays
-						PhantomArrayListCheckout checkout = pool.phantomRefToCheckout.remove(phantomRef);
-						if (checkout != null)
-						{
-							returnedByteArrayCount += checkout.getByteArrayCount();
-							returnedShortArrayCount += checkout.getShortArrayCount();
-							returnedLongArrayCount += checkout.getLongArrayCount();
-							pool.returnCheckout(checkout);
-						}
-						else
-						{
-							// shouldn't happen, but just in case
-							LOGGER.warn("Pool: ["+pool.name+"]. Unable to find checkout for phantom reference ["+phantomRef+"], arrays will need to be recreated.");
-						}
-						
-						phantomRef = pool.phantomRefQueue.poll();
-					}
-					
-					if (LOG_ARRAY_RECOVERY)
-					{
-						if (returnedByteArrayCount != 0
-								&& returnedShortArrayCount != 0
-								&& returnedLongArrayCount != 0)
-						{
-							// we only want to log when arrays have been returned
-							LOGGER.info("Pool: ["+pool.name+"]. Returned byte:["+F3Screen.NUMBER_FORMAT.format(returnedByteArrayCount)+"], short:["+F3Screen.NUMBER_FORMAT.format(returnedShortArrayCount)+"], long:["+F3Screen.NUMBER_FORMAT.format(returnedLongArrayCount)+"].");
-						}
-					}
-					
-					// since this is just for debugging it only needs to be recalculated once in a while
-					pool.recalculateSizeForDebugging();
-				}
-			}
-			catch (Exception e)
-			{
-				LOGGER.error("Unexpected error in phantom pool return thread, error: [" + e.getMessage() + "].", e);
-			}
-		}
-	}
 	
 	
-	public PhantomArrayListPool(String name)
+	public PhantomArrayListPool(String name) { this(name, false); }
+	public PhantomArrayListPool(String name, boolean logGarbageCollectedStacks)
 	{
 		POOL_LIST.add(this);
 		this.name = name;
+		this.logGarbageCollectedStacks = logGarbageCollectedStacks;
 	}
 	
 	
 	
-	
-	
 	//==============//
 	// get checkout //
 	//==============//
@@ -323,6 +270,137 @@ public class PhantomArrayListPool
 	
 	
 	
+	//==================//
+	// phantom recovery //
+	//==================//
+	
+	private static void runPhantomReferenceCleanupLoop()
+	{
+		while (true)
+		{
+			// these arrays are stored here so they don't have to be re-allocated each loop
+			ArrayList<Pair<String, AtomicInteger>> allocationStackTraceCountPairList = new ArrayList<>();
+			
+			try
+			{
+				try
+				{
+					Thread.sleep(PHANTOM_REF_CHECK_TIME_IN_MS);
+				}
+				catch (InterruptedException ignore) { }
+				
+				
+				for (int poolIndex = 0; poolIndex < POOL_LIST.size(); poolIndex++)
+				{
+					PhantomArrayListPool pool = POOL_LIST.get(poolIndex);
+					
+					int returnedByteArrayCount = 0;
+					int returnedShortArrayCount = 0;
+					int returnedLongArrayCount = 0;
+					int checkoutCount = 0;
+					
+					allocationStackTraceCountPairList.clear();
+					
+					Reference<? extends PhantomArrayListParent> phantomRef = pool.phantomRefQueue.poll();
+					while (phantomRef != null)
+					{
+						// return the pooled arrays
+						PhantomArrayListCheckout checkout = pool.phantomRefToCheckout.remove(phantomRef);
+						if (checkout != null)
+						{
+							returnedByteArrayCount += checkout.getByteArrayCount();
+							returnedShortArrayCount += checkout.getShortArrayCount();
+							returnedLongArrayCount += checkout.getLongArrayCount();
+							checkoutCount++;
+							pool.returnCheckout(checkout);
+							
+							if (pool.logGarbageCollectedStacks
+									&& checkout.allocationStackTrace != null) // stack trace shouldn't be null, but just in case
+							{
+								putAndIncrementTrackingString(checkout.allocationStackTrace, allocationStackTraceCountPairList);
+							}
+						}
+						else
+						{
+							// shouldn't happen, but just in case
+							LOGGER.warn("Pool: ["+pool.name+"]. Unable to find checkout for phantom reference ["+phantomRef+"], arrays will need to be recreated.");
+						}
+						
+						phantomRef = pool.phantomRefQueue.poll();
+					}
+					
+					if (LOG_ARRAY_RECOVERY || pool.logGarbageCollectedStacks)
+					{
+						// we only want to log when something has been returned
+						if (checkoutCount != 0
+								|| returnedByteArrayCount != 0
+								|| returnedShortArrayCount != 0
+								|| returnedLongArrayCount != 0)
+						{
+							LOGGER.warn("Pool: ["+ pool.name+"] phantom recovery. Returned checkouts:["+F3Screen.NUMBER_FORMAT.format(checkoutCount)+"], byte:["+F3Screen.NUMBER_FORMAT.format(returnedByteArrayCount)+"], short:["+F3Screen.NUMBER_FORMAT.format(returnedShortArrayCount)+"], long:["+F3Screen.NUMBER_FORMAT.format(returnedLongArrayCount)+"].");
+							
+							// log stack traces if present
+							if (pool.logGarbageCollectedStacks)
+							{
+								// high numbers first
+								allocationStackTraceCountPairList.sort((a, b) -> Integer.compare(b.second.get(), a.second.get()));
+								
+								StringBuilder stringBuilder = new StringBuilder();
+								for (int j = 0; j < allocationStackTraceCountPairList.size(); j++)
+								{
+									int count = allocationStackTraceCountPairList.get(j).second.get();
+									String stack = allocationStackTraceCountPairList.get(j).first;
+									
+									stringBuilder.append(count).append(". ").append(stack).append("\n");
+								}
+								LOGGER.warn("Stacks: ["+ allocationStackTraceCountPairList.size()+"]\n" + stringBuilder.toString());
+							}
+						}
+					}
+					
+					// since this is just for debugging it only needs to be recalculated once in a while
+					pool.recalculateSizeForDebugging();
+				}
+			}
+			catch (Exception e)
+			{
+				LOGGER.error("Unexpected error in phantom pool return thread, error: [" + e.getMessage() + "].", e);
+			}
+		}
+	}
+	/**
+	 * This was separated out so it could be used for other string pair lists.
+	 * James originally had an idea to add a shorter static string
+	 * ID to each allocated {@link PhantomArrayListCheckout} as a simpler version of the stack trace,
+	 * however it became a bit more difficult and messy than he wanted to deal with, so for now we just
+	 * have the stack trace.
+	 */
+	private static void putAndIncrementTrackingString(
+			String key,
+			ArrayList<Pair<String, AtomicInteger>> allocationStackTraceCountPairList)
+	{
+		// sequential search, for the number of elements we're dealing with (less than 20)
+		// this should be sufficiently fast
+		boolean pairFound = false;
+		for (int i = 0; i < allocationStackTraceCountPairList.size(); i++)
+		{
+			Pair<String, AtomicInteger> possiblePair = allocationStackTraceCountPairList.get(i);
+			if (possiblePair.first.equals(key))
+			{
+				possiblePair.second.getAndIncrement();
+				pairFound = true;
+				break;
+			}
+		}
+		
+		if (!pairFound)
+		{
+			allocationStackTraceCountPairList.add(new Pair<>(key, new AtomicInteger(1)));
+		}
+	}
+	
+	
+	
 	//=================//
 	// return checkout //
 	//=================//