From 7ad7a0c477956da230fc4bcd7ae0753c53bb4def Mon Sep 17 00:00:00 2001 From: austin Date: Thu, 15 May 2025 10:54:33 -0400 Subject: [PATCH] safe executor service creation --- docs/src/progress/renderertodo.md | 1 + .../ui/menu/debug/render/ImGuiRenderer.java | 16 ++++ .../engine/threads/ThreadManager.java | 75 ++++++++++++++----- .../gridded/GriddedDataCellLoaderService.java | 14 +++- .../gridded/GriddedDataCellManager.java | 44 ++++++----- 5 files changed, 108 insertions(+), 42 deletions(-) diff --git a/docs/src/progress/renderertodo.md b/docs/src/progress/renderertodo.md index 4ebf4330..c1fb79b8 100644 --- a/docs/src/progress/renderertodo.md +++ b/docs/src/progress/renderertodo.md @@ -1796,6 +1796,7 @@ Support for freeing shaders Utilities to free all of a type of resource Slowdown entity tests to prevent VSCode from exploding when running tests Fix static state caching between tests in visual shader construction +Safe executor service creation in non final static contexts diff --git a/src/main/java/electrosphere/client/ui/menu/debug/render/ImGuiRenderer.java b/src/main/java/electrosphere/client/ui/menu/debug/render/ImGuiRenderer.java index 3c489918..1d47eb82 100644 --- a/src/main/java/electrosphere/client/ui/menu/debug/render/ImGuiRenderer.java +++ b/src/main/java/electrosphere/client/ui/menu/debug/render/ImGuiRenderer.java @@ -39,6 +39,22 @@ public class ImGuiRenderer { } ImGui.unindent(); } + if(ImGui.collapsingHeader("Debug Toggles")){ + ImGui.indent(); + if(ImGui.button("Draw Client Hitboxes")){ + Globals.userSettings.setGraphicsDebugDrawCollisionSpheresClient(!Globals.userSettings.getGraphicsDebugDrawCollisionSpheresClient()); + } + if(ImGui.button("Draw Server Hitboxes")){ + Globals.userSettings.setGraphicsDebugDrawCollisionSpheresServer(!Globals.userSettings.getGraphicsDebugDrawCollisionSpheresServer()); + } + if(ImGui.button("Draw Physics Objects")){ + Globals.userSettings.setGraphicsDebugDrawPhysicsObjects(!Globals.userSettings.graphicsDebugDrawPhysicsObjects()); + } + if(ImGui.button("Draw Grid Alignment Data")){ + Globals.userSettings.setGraphicsDebugDrawGridAlignment(!Globals.userSettings.getGraphicsDebugDrawGridAlignment()); + } + ImGui.unindent(); + } if(ImGui.collapsingHeader("OpenGL Details")){ ImGui.text("GL_MAX_TEXTURE_IMAGE_UNITS: " + Globals.renderingEngine.getOpenGLContext().getMaxTextureImageUnits()); ImGui.text("GL_MAX_TEXTURE_SIZE: " + Globals.renderingEngine.getOpenGLContext().getMaxTextureSize()); diff --git a/src/main/java/electrosphere/engine/threads/ThreadManager.java b/src/main/java/electrosphere/engine/threads/ThreadManager.java index 33504522..543653a2 100644 --- a/src/main/java/electrosphere/engine/threads/ThreadManager.java +++ b/src/main/java/electrosphere/engine/threads/ThreadManager.java @@ -3,8 +3,10 @@ package electrosphere.engine.threads; import java.util.Collections; import java.util.LinkedList; import java.util.List; -import java.util.concurrent.Semaphore; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; import electrosphere.client.terrain.foliage.FoliageModel; import electrosphere.engine.Globals; @@ -13,7 +15,6 @@ import electrosphere.engine.threads.LabeledThread.ThreadLabel; import electrosphere.entity.types.terrain.BlockChunkEntity; import electrosphere.entity.types.terrain.TerrainChunk; import electrosphere.server.ai.services.PathfindingService; -import electrosphere.server.datacell.gridded.GriddedDataCellLoaderService; import electrosphere.util.CodeUtils; /** @@ -21,24 +22,37 @@ import electrosphere.util.CodeUtils; */ public class ThreadManager { - //Threadsafes the manager - Semaphore threadLock; + /** + * Threadsafes the manager + */ + private ReentrantLock threadLock; - //All threads that are actively running + /** + * All threads that are actively running + */ private List activeThreads; - //All loading threads that are actively running + /** + * All loading threads that are actively running + */ private List loadingThreads; - //Used by main thread to alert other threads whether they should keep running or not + /** + * Used by main thread to alert other threads whether they should keep running or not + */ private boolean shouldKeepRunning; + /** + * Tracks all executors created + */ + private static List executors = new LinkedList(); + /** * Initializes the thread manager */ public void init(){ - threadLock = new Semaphore(1); + threadLock = new ReentrantLock(); activeThreads = new LinkedList(); loadingThreads = new LinkedList(); shouldKeepRunning = true; @@ -48,7 +62,7 @@ public class ThreadManager { * Updates what threads are being tracked */ public void update(){ - threadLock.acquireUninterruptibly(); + threadLock.lock(); // //remove loading threads List threadsToRemove = new LinkedList(); @@ -61,7 +75,7 @@ public class ThreadManager { loadingThreads.remove(thread); } - threadLock.release(); + threadLock.unlock(); } /** @@ -69,10 +83,10 @@ public class ThreadManager { * @param thread The thread to start */ public void start(ThreadLabel label, Thread thread){ - threadLock.acquireUninterruptibly(); + threadLock.lock(); activeThreads.add(new LabeledThread(label, thread)); thread.start(); - threadLock.release(); + threadLock.unlock(); } /** @@ -80,11 +94,11 @@ public class ThreadManager { * @param thread The loading thread to start */ public void start(LoadingThread thread){ - threadLock.acquireUninterruptibly(); + threadLock.lock(); activeThreads.add(new LabeledThread(ThreadLabel.LOADING, thread)); loadingThreads.add(thread); thread.start(); - threadLock.release(); + threadLock.unlock(); } /** @@ -108,7 +122,7 @@ public class ThreadManager { */ public void close(){ this.shouldKeepRunning = false; - threadLock.acquireUninterruptibly(); + threadLock.lock(); //for some reason, server must be explicitly closed if(Globals.server != null){ @@ -125,9 +139,15 @@ public class ThreadManager { TerrainChunk.haltThreads(); FoliageModel.haltThreads(); BlockChunkEntity.haltThreads(); - GriddedDataCellLoaderService.haltThreads(); PathfindingService.haltThreads(); + /** + * Halt all requested executors + */ + for(ExecutorService service : executors){ + service.shutdownNow(); + } + // //interrupt all threads for(int i = 0; i < 3; i++){ @@ -150,7 +170,7 @@ public class ThreadManager { CodeUtils.todo(e, "Handle failing to sleep while interrupting all other threads"); } } - threadLock.release(); + threadLock.unlock(); } /** @@ -166,7 +186,7 @@ public class ThreadManager { * @param label The label */ public void interruptLabel(ThreadLabel label){ - threadLock.acquireUninterruptibly(); + threadLock.lock(); // //interrupt threads for(LabeledThread thread : activeThreads){ @@ -174,7 +194,24 @@ public class ThreadManager { thread.getThread().interrupt(); } } - threadLock.release(); + threadLock.unlock(); + } + + /** + * Requests a fixed-size thread pool + * @param threads The number of threads + * @return The executor + */ + public ExecutorService requestFixedThreadPool(int threads){ + if(threads < 1){ + throw new Error("Requested invalid number of threads! " + threads); + } + ExecutorService rVal = null; + threadLock.lock(); + rVal = Executors.newFixedThreadPool(threads); + executors.add(rVal); + threadLock.unlock(); + return rVal; } } diff --git a/src/main/java/electrosphere/server/datacell/gridded/GriddedDataCellLoaderService.java b/src/main/java/electrosphere/server/datacell/gridded/GriddedDataCellLoaderService.java index 97d18f12..51f5c6b1 100644 --- a/src/main/java/electrosphere/server/datacell/gridded/GriddedDataCellLoaderService.java +++ b/src/main/java/electrosphere/server/datacell/gridded/GriddedDataCellLoaderService.java @@ -4,7 +4,6 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.locks.ReentrantLock; @@ -20,7 +19,7 @@ public class GriddedDataCellLoaderService { /** * Used for loading/unloading the cells */ - protected static final ExecutorService ioThreadService = Executors.newFixedThreadPool(ThreadCounts.GRIDDED_DATACELL_LOADING_THREADS); + private ExecutorService ioThreadService = null; /** * Lock for structures in this service @@ -37,13 +36,20 @@ public class GriddedDataCellLoaderService { */ private static final Map jobOperationMap = new HashMap(); + /** + * Constructor + */ + public GriddedDataCellLoaderService(){ + this.ioThreadService = Globals.threadManager.requestFixedThreadPool(ThreadCounts.GRIDDED_DATACELL_LOADING_THREADS); + } + /** * Queues an operation that requires a read or write of a location's file data. * Guarantees that all operations will be run in order without losing any work. * @param key The key for the cell * @param operation The operation to perform */ - protected static void queueLocationBasedOperation(long key, Runnable operation){ + protected void queueLocationBasedOperation(long key, Runnable operation){ lock.lock(); //if there is a job queued and we couldn't cancel it, wait Future job = queuedWorkLock.get(key); @@ -96,7 +102,7 @@ public class GriddedDataCellLoaderService { /** * Halts the threads for the data cell loader service */ - public static void haltThreads(){ + public void haltThreads(){ ioThreadService.shutdownNow(); } diff --git a/src/main/java/electrosphere/server/datacell/gridded/GriddedDataCellManager.java b/src/main/java/electrosphere/server/datacell/gridded/GriddedDataCellManager.java index 80f2faca..768a9c87 100644 --- a/src/main/java/electrosphere/server/datacell/gridded/GriddedDataCellManager.java +++ b/src/main/java/electrosphere/server/datacell/gridded/GriddedDataCellManager.java @@ -9,7 +9,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.Semaphore; import java.util.concurrent.locks.ReentrantLock; @@ -76,16 +75,21 @@ public class GriddedDataCellManager implements DataCellManager, VoxelCellManager */ static final double SIMULATION_DISTANCE_CUTOFF = 5; - /** - * Used for generating physics chunks - */ - static final ExecutorService generationService = Executors.newFixedThreadPool(ThreadCounts.GRIDDED_DATACELL_PHYSICS_GEN_THREADS); - /** * Big number used when scanning for a data cell to spawn a macro object within */ static final double MACRO_SCANNING_BIG_NUMBER = 10000; + /** + * Used for generating physics chunks + */ + ExecutorService generationService = null; + + /** + * The service for loading data cells from disk + */ + GriddedDataCellLoaderService loaderService; + /** * Tracks whether this manager has been flagged to unload cells or not */ @@ -192,15 +196,17 @@ public class GriddedDataCellManager implements DataCellManager, VoxelCellManager this.serverFluidManager == null || this.serverContentManager == null ){ - throw new IllegalStateException("Tried to create a GriddedDataCellManager with invalid parameters " + - this.parent + " " + - this.serverWorldData + " " + - this.serverTerrainManager + " " + - this.serverFluidManager + " " + - this.serverContentManager + " " + throw new Error("Tried to create a GriddedDataCellManager with invalid parameters " + + this.parent + " " + + this.serverWorldData + " " + + this.serverTerrainManager + " " + + this.serverFluidManager + " " + + this.serverContentManager + " " ); } this.pathfinder = new VoxelPathfinder(); + this.loaderService = new GriddedDataCellLoaderService(); + this.generationService = Globals.threadManager.requestFixedThreadPool(ThreadCounts.GRIDDED_DATACELL_PHYSICS_GEN_THREADS); } /** @@ -549,7 +555,7 @@ public class GriddedDataCellManager implements DataCellManager, VoxelCellManager //terrain is saved before tracking is removed. This makes sure that any side effects from calling savePositionToDisk (ie if it looks up the chunk that we're deleting) //don't trigger the chunk to be re-created Globals.profiler.beginCpuSample("GriddedDataCellManager.unloadPlayerlessChunks - Store data"); - GriddedDataCellLoaderService.queueLocationBasedOperation(key, () -> { + this.loaderService.queueLocationBasedOperation(key, () -> { serverContentManager.saveSerializationToDisk(key, serializedEntities); serverTerrainManager.savePositionToDisk(worldPos); }); @@ -768,7 +774,7 @@ public class GriddedDataCellManager implements DataCellManager, VoxelCellManager * Because cell hasn't been registered yet, no simulation is performed until the physics is created. * @param worldPos */ - private static void runPhysicsGenerationThread( + private void runPhysicsGenerationThread( Vector3i worldPos, Long key, PhysicsDataCell cell, @@ -805,7 +811,7 @@ public class GriddedDataCellManager implements DataCellManager, VoxelCellManager ServerEntityUtils.destroyEntity(blockEntity); } - generationService.submit(() -> { + this.generationService.submit(() -> { try { BlockChunkData blockChunkData = realm.getServerWorldData().getServerBlockManager().getChunk(worldPos.x, worldPos.y, worldPos.z); ServerTerrainChunk terrainChunk = realm.getServerWorldData().getServerTerrainManager().getChunk(worldPos.x, worldPos.y, worldPos.z, ServerChunkCache.STRIDE_FULL_RES); @@ -861,7 +867,7 @@ public class GriddedDataCellManager implements DataCellManager, VoxelCellManager Long key = this.getServerDataCellKey(localWorldPos); //generate content - GriddedDataCellLoaderService.queueLocationBasedOperation(key, () -> { + this.loaderService.queueLocationBasedOperation(key, () -> { try { serverContentManager.generateContentForDataCell(parent, localWorldPos, rVal, cellKey); } catch(Error e){ @@ -873,7 +879,7 @@ public class GriddedDataCellManager implements DataCellManager, VoxelCellManager //generates physics for the cell in a dedicated thread then finally registers loadedCellsLock.lock(); PhysicsDataCell cell = posPhysicsMap.get(key); - GriddedDataCellManager.runPhysicsGenerationThread(localWorldPos,key,cell,this.posPhysicsMap,this.groundDataCells,this.cellTrackingMap,this.parent); + this.runPhysicsGenerationThread(localWorldPos,key,cell,this.posPhysicsMap,this.groundDataCells,this.cellTrackingMap,this.parent); loadedCellsLock.unlock(); @@ -1047,8 +1053,8 @@ public class GriddedDataCellManager implements DataCellManager, VoxelCellManager * Stops the executor service */ public void halt(){ - generationService.shutdownNow(); - GriddedDataCellLoaderService.ioThreadService.shutdownNow(); + this.generationService.shutdownNow(); + this.loaderService.haltThreads(); } @Override