From 046b38992527e5abccfda8aed88496635081d287 Mon Sep 17 00:00:00 2001 From: Brian Neumann-Fopiano Date: Sun, 6 Sep 2026 04:07:51 -0400 Subject: [PATCH] Completed shutdown retry ownership - Mark generation modes closed only after every stage finishes cleanup - Retain manager loop and cleanup executor ownership until their existing shutdown waits succeed - Propagate manager shutdown timeouts and interruption so resource release remains blocked - Track unpublished runtimes by identity while resource state changes during cleanup - Verify failed joins, interrupted waits, stage retries, and changing resource hashes --- .../iris/engine/EngineShutdownSequence.java | 7 +- .../arcane/iris/engine/IrisWorldManager.java | 20 +-- .../iris/engine/framework/IrisEngineMode.java | 2 +- .../iris/engine/EngineShutdownDrainTest.java | 31 ++++- .../engine/IrisWorldManagerShutdownTest.java | 127 ++++++++++++++++++ .../IrisEngineModeLifecycleTest.java | 57 ++++++++ 6 files changed, 230 insertions(+), 14 deletions(-) create mode 100644 core/src/test/java/art/arcane/iris/engine/IrisWorldManagerShutdownTest.java create mode 100644 core/src/test/java/art/arcane/iris/engine/framework/IrisEngineModeLifecycleTest.java diff --git a/core/src/main/java/art/arcane/iris/engine/EngineShutdownSequence.java b/core/src/main/java/art/arcane/iris/engine/EngineShutdownSequence.java index 482ddb263..70545c13d 100644 --- a/core/src/main/java/art/arcane/iris/engine/EngineShutdownSequence.java +++ b/core/src/main/java/art/arcane/iris/engine/EngineShutdownSequence.java @@ -45,7 +45,8 @@ final class EngineShutdownSequence { private final IrisEngine engine; private final Set incompleteAssemblies = ConcurrentHashMap.newKeySet(); - private final Set unpublishedRuntimes = ConcurrentHashMap.newKeySet(); + private final Set unpublishedRuntimes = Collections.synchronizedSet( + Collections.newSetFromMap(new IdentityHashMap<>())); private final Set incompleteTargets = Collections.synchronizedSet( Collections.newSetFromMap(new IdentityHashMap<>())); private boolean runtimeReleased; @@ -267,7 +268,7 @@ final class EngineShutdownSequence { return true; } } - for (EngineRuntime runtime : unpublishedRuntimes) { + for (EngineRuntime runtime : unpublishedRuntimes.toArray(new EngineRuntime[0])) { if (runtime.generation().data() == data) { return true; } @@ -351,7 +352,7 @@ final class EngineShutdownSequence { new IllegalStateException("Iris runtime assembly still owns the detached target.")); } } - for (EngineRuntime runtime : unpublishedRuntimes) { + for (EngineRuntime runtime : unpublishedRuntimes.toArray(new EngineRuntime[0])) { if (runtime.generation().target() == target) { return appendFailure(failure, new IllegalStateException("Iris unpublished runtime still owns the detached target.")); diff --git a/core/src/main/java/art/arcane/iris/engine/IrisWorldManager.java b/core/src/main/java/art/arcane/iris/engine/IrisWorldManager.java index af77a1014..a10cbec55 100644 --- a/core/src/main/java/art/arcane/iris/engine/IrisWorldManager.java +++ b/core/src/main/java/art/arcane/iris/engine/IrisWorldManager.java @@ -277,26 +277,26 @@ public class IrisWorldManager extends EngineAssignedWorldManager { } if (!looperStopped) { try { - looperStopped = true; if (looper != null) { looper.interrupt(); - joinQuietly(looper); + awaitLooper(looper); } + looperStopped = true; } catch (Throwable e) { failure = appendCloseFailure(failure, e); } } if (!cleanupServiceStopped) { try { - cleanupServiceStopped = true; if (cleanupService != null) { for (Future future : cleanup.values()) { future.cancel(false); } cleanup.clear(); cleanupService.shutdownNow(); - awaitQuietly(cleanupService); + awaitCleanupService(cleanupService); } + cleanupServiceStopped = true; } catch (Throwable e) { failure = appendCloseFailure(failure, e); } @@ -306,28 +306,30 @@ public class IrisWorldManager extends EngineAssignedWorldManager { } } - private void joinQuietly(Thread thread) { + private void awaitLooper(Thread thread) { if (thread == Thread.currentThread()) { - return; + throw new IllegalStateException("Cannot finish world manager shutdown from its own loop thread."); } try { thread.join(CLOSE_AWAIT_MS); if (thread.isAlive()) { - IrisLogging.warn("Thread " + thread.getName() + " did not stop within " + CLOSE_AWAIT_MS + "ms."); + throw new IllegalStateException("Thread " + thread.getName() + " did not stop within " + CLOSE_AWAIT_MS + "ms."); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted while waiting for the world manager loop to stop.", e); } } - private void awaitQuietly(ScheduledExecutorService service) { + private void awaitCleanupService(ScheduledExecutorService service) { try { if (!service.awaitTermination(CLOSE_AWAIT_MS, TimeUnit.MILLISECONDS)) { - IrisLogging.warn("Mantle cleanup executor did not stop within " + CLOSE_AWAIT_MS + "ms."); + throw new IllegalStateException("Mantle cleanup executor did not stop within " + CLOSE_AWAIT_MS + "ms."); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted while waiting for mantle cleanup to stop.", e); } } diff --git a/core/src/main/java/art/arcane/iris/engine/framework/IrisEngineMode.java b/core/src/main/java/art/arcane/iris/engine/framework/IrisEngineMode.java index 9112f589f..9ccdb0be2 100644 --- a/core/src/main/java/art/arcane/iris/engine/framework/IrisEngineMode.java +++ b/core/src/main/java/art/arcane/iris/engine/framework/IrisEngineMode.java @@ -42,11 +42,11 @@ public abstract class IrisEngineMode implements EngineMode { return; } - closed = true; dump(); terrainStages.forEach(EngineStage::close); terrainStages.clear(); transitionStage.close(); + closed = true; } @Override diff --git a/core/src/test/java/art/arcane/iris/engine/EngineShutdownDrainTest.java b/core/src/test/java/art/arcane/iris/engine/EngineShutdownDrainTest.java index fe8e6f2ed..2c82e62c3 100644 --- a/core/src/test/java/art/arcane/iris/engine/EngineShutdownDrainTest.java +++ b/core/src/test/java/art/arcane/iris/engine/EngineShutdownDrainTest.java @@ -2,6 +2,8 @@ package art.arcane.iris.engine; import art.arcane.iris.core.loader.IrisData; import art.arcane.iris.engine.EngineRuntimeBuilder.RuntimeAssembly; +import art.arcane.iris.engine.framework.Engine; +import art.arcane.iris.engine.framework.EngineAssignedComponent; import art.arcane.iris.engine.framework.EngineEffects; import art.arcane.iris.engine.framework.EnginePlatformHooks; import art.arcane.iris.engine.framework.EngineTarget; @@ -251,7 +253,8 @@ public class EngineShutdownDrainTest { ShutdownFixture fixture = new ShutdownFixture(); EngineTarget nextTarget = target(); GenerationRuntime nextGeneration = generation(nextTarget, fixture.mantle); - EngineRuntime next = new EngineRuntime(nextGeneration, mock(EngineEffects.class), mock(EngineWorldManager.class)); + EngineRuntime next = new EngineRuntime(nextGeneration, + new ChangingEffects(fixture.engine), mock(EngineWorldManager.class)); EngineMode nextMode = nextGeneration.mode(); doThrow(new IllegalStateException("Previous planner active")).when(fixture.complex).close(); doThrow(new IllegalStateException("Next mode active")).when(nextMode).close(); @@ -320,6 +323,32 @@ public class EngineShutdownDrainTest { field.set(target, value); } + private static final class ChangingEffects extends EngineAssignedComponent implements EngineEffects { + private int hash = 1; + + private ChangingEffects(Engine engine) { + super(engine, "changing effects"); + } + + @Override + public void updatePlayerMap() { + } + + @Override + public void tickRandomPlayer() { + } + + @Override + public void close() { + hash++; + } + + @Override + public int hashCode() { + return hash; + } + } + private static final class ShutdownFixture { private final IrisEngine engine = mock(IrisEngine.class); private final EngineTarget target = target(); diff --git a/core/src/test/java/art/arcane/iris/engine/IrisWorldManagerShutdownTest.java b/core/src/test/java/art/arcane/iris/engine/IrisWorldManagerShutdownTest.java new file mode 100644 index 000000000..fe7854ea3 --- /dev/null +++ b/core/src/test/java/art/arcane/iris/engine/IrisWorldManagerShutdownTest.java @@ -0,0 +1,127 @@ +package art.arcane.iris.engine; + +import art.arcane.iris.engine.framework.Engine; +import art.arcane.iris.engine.framework.EngineTarget; +import art.arcane.iris.engine.object.IrisWorld; +import art.arcane.volmlib.util.scheduling.Looper; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class IrisWorldManagerShutdownTest { + @Test + public void liveLooperPreventsSuccessfulCloseAndRemainsRetryable() throws Exception { + IrisWorldManager manager = manager(); + HeldLooper looper = new HeldLooper(); + setField(manager, "looper", looper); + looper.start(); + try { + assertTrue(looper.started.await(5L, TimeUnit.SECONDS)); + assertThrows(IllegalStateException.class, manager::close); + assertFalse(manager.isLooperStopped()); + assertTrue(looper.isAlive()); + assertTrue(manager.isCleanupServiceStopped()); + } finally { + looper.release.countDown(); + looper.join(5_000L); + manager.close(); + } + assertFalse(looper.isAlive()); + assertTrue(manager.isLooperStopped()); + manager.close(); + } + + @Test + public void cleanupTimeoutRemainsRetryableAndSuccessfulCloseIsIdempotent() throws Exception { + IrisWorldManager manager = manager(); + ScheduledThreadPoolExecutor executor = mock(ScheduledThreadPoolExecutor.class); + manager.getCleanupService().shutdownNow(); + setField(manager, "cleanupService", executor); + when(executor.awaitTermination(anyLong(), eq(TimeUnit.MILLISECONDS))).thenReturn(false, true); + + assertThrows(IllegalStateException.class, manager::close); + assertFalse(manager.isCleanupServiceStopped()); + assertTrue(manager.isLooperStopped()); + + manager.close(); + assertTrue(manager.isCleanupServiceStopped()); + manager.close(); + verify(executor, times(2)).shutdownNow(); + verify(executor, times(2)).awaitTermination(anyLong(), eq(TimeUnit.MILLISECONDS)); + } + + @Test + public void interruptedCleanupWaitRestoresInterruptAndCanBeRetried() throws Exception { + IrisWorldManager manager = manager(); + ScheduledThreadPoolExecutor executor = mock(ScheduledThreadPoolExecutor.class); + manager.getCleanupService().shutdownNow(); + setField(manager, "cleanupService", executor); + when(executor.awaitTermination(anyLong(), eq(TimeUnit.MILLISECONDS))) + .thenThrow(new InterruptedException("Close interrupted")) + .thenReturn(true); + try { + assertThrows(IllegalStateException.class, manager::close); + assertTrue(Thread.currentThread().isInterrupted()); + assertFalse(manager.isCleanupServiceStopped()); + } finally { + Thread.interrupted(); + manager.close(); + } + assertTrue(manager.isCleanupServiceStopped()); + verify(executor, times(2)).awaitTermination(anyLong(), eq(TimeUnit.MILLISECONDS)); + } + + private static IrisWorldManager manager() { + Engine engine = mock(Engine.class); + EngineTarget target = mock(EngineTarget.class); + IrisWorld world = mock(IrisWorld.class); + when(world.name()).thenReturn("shutdown-test"); + when(target.getWorld()).thenReturn(world); + when(engine.getTarget()).thenReturn(target); + return new IrisWorldManager(engine); + } + + private static void setField(IrisWorldManager manager, String name, Object value) throws Exception { + Field field = IrisWorldManager.class.getDeclaredField(name); + field.setAccessible(true); + field.set(manager, value); + } + + private static final class HeldLooper extends Looper { + private final CountDownLatch started = new CountDownLatch(1); + private final CountDownLatch release = new CountDownLatch(1); + + @Override + protected void onStart() { + } + + @Override + protected void onStop() { + } + + @Override + protected long loop() { + started.countDown(); + while (true) { + try { + release.await(); + return -1L; + } catch (InterruptedException ignored) { + } + } + } + } +} diff --git a/core/src/test/java/art/arcane/iris/engine/framework/IrisEngineModeLifecycleTest.java b/core/src/test/java/art/arcane/iris/engine/framework/IrisEngineModeLifecycleTest.java new file mode 100644 index 000000000..81f5f7c4c --- /dev/null +++ b/core/src/test/java/art/arcane/iris/engine/framework/IrisEngineModeLifecycleTest.java @@ -0,0 +1,57 @@ +package art.arcane.iris.engine.framework; + +import org.junit.Test; + +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; + +public class IrisEngineModeLifecycleTest { + @Test + public void failedContentStageRemainsRetryable() { + IrisEngineMode mode = new IrisEngineMode(mock(Engine.class)) { + }; + EngineStage content = mock(EngineStage.class); + EngineStage terrain = mock(EngineStage.class); + mode.registerStage(content); + mode.registerTerrainStage(terrain); + IllegalStateException failure = new IllegalStateException("Stage close failed"); + doThrow(failure).doNothing().when(content).close(); + + assertSame(failure, assertThrows(IllegalStateException.class, mode::close)); + verifyNoInteractions(terrain); + mode.close(); + mode.close(); + + assertTrue(mode.getStages().isEmpty()); + assertTrue(mode.getTerrainStages().isEmpty()); + verify(content, times(2)).close(); + verify(terrain).close(); + } + + @Test + public void failedTerrainStageDoesNotRepeatReleasedContent() { + IrisEngineMode mode = new IrisEngineMode(mock(Engine.class)) { + }; + EngineStage content = mock(EngineStage.class); + EngineStage terrain = mock(EngineStage.class); + mode.registerStage(content); + mode.registerTerrainStage(terrain); + IllegalStateException failure = new IllegalStateException("Terrain stage close failed"); + doThrow(failure).doNothing().when(terrain).close(); + + assertSame(failure, assertThrows(IllegalStateException.class, mode::close)); + assertTrue(mode.getStages().isEmpty()); + mode.close(); + mode.close(); + + assertTrue(mode.getTerrainStages().isEmpty()); + verify(content).close(); + verify(terrain, times(2)).close(); + } +}