Refactor AsynchronousSceneManager to use a single thread ExecutorService.

chunky/src/java/se/llbit/chunky/main/Chunky.java

@@ -375,7 +375,6 @@ public RenderController getRenderController() {
         SceneProvider sceneProvider = sceneManager.getSceneProvider();
         renderManager.setSceneProvider(sceneProvider);
         renderManager.start();
-        sceneManager.start();
         renderController = new RenderController(context, renderManager, sceneManager, sceneProvider);
       }
     }

chunky/src/java/se/llbit/chunky/renderer/scene/AsynchronousSceneManager.java

@@ -30,7 +30,8 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.Collection;
-import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 
 /**
  * This scene manager is used for asynchronous loading and saving of scenes.
@@ -39,16 +40,39 @@
  *
  * @author Jesper Öqvist <[email protected]>
  */
-public class AsynchronousSceneManager extends Thread implements SceneManager {
+public class AsynchronousSceneManager implements SceneManager {
 
   private final SynchronousSceneManager sceneManager;
-  private final LinkedBlockingQueue<Runnable> taskQueue;
+  private final ExecutorService executorService = Executors.newSingleThreadExecutor(runnable -> {
+    Thread thread = new Thread(runnable, "AsynchronousSceneManager-Thread");
+    thread.setUncaughtExceptionHandler((t, exception) -> {
+      if (exception instanceof OutOfMemoryError) {
+        Log.error(
+          "Chunky has run out of memory! Increase the memory given to Chunky in the launcher.",
+          exception);
+        throw (Error) exception;
+      } else if(exception instanceof InterruptedException) {
+        Thread.currentThread().interrupt();
+      } else {
+        Log.error("Scene manager has crashed due to an uncaught exception. \n" +
+          "Chunky might not work properly until you restart it.\n" +
+          "If you think this is a bug, please report it and the following lines to the developers.",
+          exception);
+        // a new thread will be created by the executor after the current one terminated and a new task is enqueued
+      }
+    });
+    return thread;
+  });
 
   public AsynchronousSceneManager(RenderContext context, RenderManager renderManager) {
-    super("Scene Manager");
-
     sceneManager = new SynchronousSceneManager(context, renderManager);
-    taskQueue = new LinkedBlockingQueue<>();
+  }
+
+  /**
+   * Schedule a task to be run soon.
+   */
+  public void enqueueTask(Runnable task) {
+    executorService.execute(task);
   }
 
   @Override
@@ -85,26 +109,6 @@ public void setOnChunksLoaded(Runnable onChunksLoaded) {
     return sceneManager.getScene();
   }
 
-  @Override public void run() {
-    try {
-      while (!isInterrupted()) {
-        Runnable task = taskQueue.take();
-        task.run();
-      }
-    } catch (InterruptedException ignored) {
-      // Interrupted.
-    } catch (Throwable e) {
-      if (e instanceof OutOfMemoryError) {
-        Log.error("Chunky has run out of memory! Increase the memory given to Chunky in the launcher.", e);
-      } else {
-        Log.error("Scene manager has crashed due to an uncaught exception. " +
-            "Chunky will not work properly until you restart it. " +
-            "If you think this is a bug, please report it to the developers.", e);
-      }
-      throw e;
-    }
-  }
-
   /**
    * Load the given scene.
    *
@@ -229,84 +233,6 @@ public void mergeRenderDump(File renderDump) {
     enqueueTask(() -> sceneManager.mergeDump(renderDump));
   }
 
-  /**
-   * Schedule a task to be run soon.
-   */
-  public void enqueueTask(Runnable task) {
-    taskQueue.add(task);
-  }
-
-  /**
-   * Remove problematic characters from scene name.
-   *
-   * @return sanitized scene name
-   */
-  public static String sanitizedSceneName(String name, String fallbackName) {
-    name = name.trim();
-    StringBuilder sb = new StringBuilder();
-    for (int i = 0; i < name.length(); ++i) {
-      char c = name.charAt(i);
-      if (isValidSceneNameChar(c)) {
-        sb.append(c);
-      } else if (c >= '\u0020' && c <= '\u007e') {
-        sb.append('_');
-      }
-    }
-    String stripped = sb.toString().trim();
-    if (stripped.isEmpty()) {
-      return fallbackName;
-    } else {
-      return stripped;
-    }
-  }
-
-  /**
-   * Remove problematic characters from scene name.
-   *
-   * @return sanitized scene name
-   */
-  public static String sanitizedSceneName(String name) {
-    return sanitizedSceneName(name, "Scene");
-  }
-
-  /**
-   * @return <code>false</code> if the character can cause problems on any
-   * supported platform.
-   */
-  public static boolean isValidSceneNameChar(char c) {
-    switch (c) {
-      case '/':
-      case ':':
-      case ';':
-      case '\\': // Windows file separator.
-      case '*':
-      case '?':
-      case '"':
-      case '<':
-      case '>':
-      case '|':
-        return false;
-    }
-    if (c < '\u0020') {
-      return false;
-    }
-    return c <= '\u007e' || c >= '\u00a0';
-  }
-
-  /**
-   * Check for scene name validity.
-   *
-   * @return <code>true</code> if the scene name contains only legal characters
-   */
-  public static boolean sceneNameIsValid(String name) {
-    for (int i = 0; i < name.length(); ++i) {
-      if (!isValidSceneNameChar(name.charAt(i))) {
-        return false;
-      }
-    }
-    return true;
-  }
-
   public void discardSceneChanges() {
     sceneManager.discardSceneChanges();
   }

chunky/src/java/se/llbit/chunky/renderer/scene/Scene.java

@@ -1841,7 +1841,7 @@ public Vector3i getOrigin() {
    * Set the scene name.
    */
   public void setName(String newName) {
-    newName = AsynchronousSceneManager.sanitizedSceneName(newName);
+    newName = SceneUtils.sanitizedSceneName(newName);
     if (newName.length() > 0) {
       name = newName;
     }

chunky/src/java/se/llbit/chunky/renderer/scene/SceneUtils.java

@@ -0,0 +1,75 @@
+package se.llbit.chunky.renderer.scene;
+
+public class SceneUtils {
+
+  /**
+   * Remove problematic characters from scene name.
+   *
+   * @return sanitized scene name
+   */
+  public static String sanitizedSceneName(String name, String fallbackName) {
+    name = name.trim();
+    StringBuilder sb = new StringBuilder();
+    for (int i = 0; i < name.length(); ++i) {
+      char c = name.charAt(i);
+      if (isValidSceneNameChar(c)) {
+        sb.append(c);
+      } else if (c >= '\u0020' && c <= '\u007e') {
+        sb.append('_');
+      }
+    }
+    String stripped = sb.toString().trim();
+    if (stripped.isEmpty()) {
+      return fallbackName;
+    } else {
+      return stripped;
+    }
+  }
+
+  /**
+   * Remove problematic characters from scene name.
+   *
+   * @return sanitized scene name
+   */
+  public static String sanitizedSceneName(String name) {
+    return sanitizedSceneName(name, "Scene");
+  }
+
+  /**
+   * @return <code>false</code> if the character can cause problems on any
+   * supported platform.
+   */
+  public static boolean isValidSceneNameChar(char c) {
+    switch (c) {
+      case '/':
+      case ':':
+      case ';':
+      case '\\': // Windows file separator.
+      case '*':
+      case '?':
+      case '"':
+      case '<':
+      case '>':
+      case '|':
+        return false;
+    }
+    if (c < '\u0020') {
+      return false;
+    }
+    return c <= '\u007e' || c >= '\u00a0';
+  }
+
+  /**
+   * Check for scene name validity.
+   *
+   * @return <code>true</code> if the scene name contains only legal characters
+   */
+  public static boolean sceneNameIsValid(String name) {
+    for (int i = 0; i < name.length(); ++i) {
+      if (!isValidSceneNameChar(name.charAt(i))) {
+        return false;
+      }
+    }
+    return true;
+  }
+}

chunky/src/java/se/llbit/chunky/ui/controller/ChunkyFxController.java

@@ -385,10 +385,10 @@ public void exportMapView() {
     saveScene.setOnAction(e -> asyncSceneManager.saveScene());
 
     saveSceneAs.setOnAction((e) -> {
-      ValidatingTextInputDialog sceneNameDialog = new ValidatingTextInputDialog(scene.name(), AsynchronousSceneManager::sceneNameIsValid);
+      ValidatingTextInputDialog sceneNameDialog = new ValidatingTextInputDialog(scene.name(), SceneUtils::sceneNameIsValid);
       sceneNameDialog.setTitle("Save scene as…");
       sceneNameDialog.setHeaderText("Enter a scene name");
-      String newName = AsynchronousSceneManager.sanitizedSceneName(sceneNameDialog.showAndWait().orElse(""), "");
+      String newName = SceneUtils.sanitizedSceneName(sceneNameDialog.showAndWait().orElse(""), "");
       if (!newName.isEmpty() && this.promptSaveScene(newName)) {
         asyncSceneManager.saveSceneAs(newName);
         scene.setName(newName);
@@ -397,10 +397,10 @@ public void exportMapView() {
     });
 
     saveSceneCopy.setOnAction((e) -> {
-      ValidatingTextInputDialog sceneNameDialog = new ValidatingTextInputDialog("Copy of " + scene.name(), AsynchronousSceneManager::sceneNameIsValid);
+      ValidatingTextInputDialog sceneNameDialog = new ValidatingTextInputDialog("Copy of " + scene.name(), SceneUtils::sceneNameIsValid);
       sceneNameDialog.setTitle("Save a copy…");
       sceneNameDialog.setHeaderText("Enter a scene name");
-      String newName = AsynchronousSceneManager.sanitizedSceneName(sceneNameDialog.showAndWait().orElse(""), "");
+      String newName = SceneUtils.sanitizedSceneName(sceneNameDialog.showAndWait().orElse(""), "");
       if (!newName.isEmpty() && this.promptSaveScene(newName)) {
         File sceneDirectory = SynchronousSceneManager.resolveSceneDirectory(newName);
         asyncSceneManager.enqueueTask(() -> {

chunky/src/test/se/llbit/chunky/renderer/scene/SceneManagerTest.java

@@ -30,41 +30,41 @@ public class SceneManagerTest {
 	 */
 	@Test
 	public void testSceneNameSanitizing_1() {
-		assertEquals("ab_cd", AsynchronousSceneManager.sanitizedSceneName("ab/cd"));
-		assertEquals("x___cd", AsynchronousSceneManager.sanitizedSceneName("x<>|cd"));
-		assertEquals("______#42", AsynchronousSceneManager.sanitizedSceneName(":*?\\\"/#42"));
+		assertEquals("ab_cd", SceneUtils.sanitizedSceneName("ab/cd"));
+		assertEquals("x___cd", SceneUtils.sanitizedSceneName("x<>|cd"));
+		assertEquals("______#42", SceneUtils.sanitizedSceneName(":*?\\\"/#42"));
 	}
 
 	/**
 	 * Whitespace is trimmed
 	 */
 	@Test
 	public void testSceneNameSanitizing_2() {
-		assertEquals("foo bar", AsynchronousSceneManager.sanitizedSceneName(" foo bar   "));
-		assertEquals("foo bar", AsynchronousSceneManager.sanitizedSceneName(" \nfoo bar\t "));
+		assertEquals("foo bar", SceneUtils.sanitizedSceneName(" foo bar   "));
+		assertEquals("foo bar", SceneUtils.sanitizedSceneName(" \nfoo bar\t "));
 	}
 
 	/**
 	 * Control characters are stripped
 	 */
 	@Test
 	public void testSceneNameSanitizing_3() {
-		assertEquals("xyzabc", AsynchronousSceneManager.sanitizedSceneName("xyz\u007fabc"));
-		assertEquals("12", AsynchronousSceneManager.sanitizedSceneName("1\u009f2"));
-		assertEquals("AZ", AsynchronousSceneManager.sanitizedSceneName("A\u0000\u0001\u001fZ"));
+		assertEquals("xyzabc", SceneUtils.sanitizedSceneName("xyz\u007fabc"));
+		assertEquals("12", SceneUtils.sanitizedSceneName("1\u009f2"));
+		assertEquals("AZ", SceneUtils.sanitizedSceneName("A\u0000\u0001\u001fZ"));
 	}
 
 	/**
 	 * Empty names are replaced by 'Scene'
 	 */
 	@Test
 	public void testSceneNameSanitizing_4() {
-		assertEquals("Scene", AsynchronousSceneManager.sanitizedSceneName(""));
-		assertEquals("Scene", AsynchronousSceneManager.sanitizedSceneName("   "));
-		assertEquals("Scene", AsynchronousSceneManager.sanitizedSceneName("\t\n\b\r"));
-		assertEquals("Scene", AsynchronousSceneManager.sanitizedSceneName("\u0080"));
+		assertEquals("Scene", SceneUtils.sanitizedSceneName(""));
+		assertEquals("Scene", SceneUtils.sanitizedSceneName("   "));
+		assertEquals("Scene", SceneUtils.sanitizedSceneName("\t\n\b\r"));
+		assertEquals("Scene", SceneUtils.sanitizedSceneName("\u0080"));
 
 		// test that whitespace is stripped after removing control chars
-		assertEquals("Scene", AsynchronousSceneManager.sanitizedSceneName("\u0085 \u0085"));
+		assertEquals("Scene", SceneUtils.sanitizedSceneName("\u0085 \u0085"));
 	}
 }