From 889241c5fb575a87037f24bc32eb92789711092a Mon Sep 17 00:00:00 2001
From: Curtis Rueden <ctrueden@wisc.edu>
Date: Fri, 25 Apr 2025 15:08:42 -0500
Subject: [PATCH] Try to be thread-safe when showing UI elements

Instead of being thread-safe in the DefaultUIService, we move the thread
safety into the AbstractUserInterface itself, since each user interface
is responsible for deciding how to behave. This retains the behavior of
showUI being thread-safe, but also makes the other ui.show methods
thread-safe as well. So now DefaultUIService doesn't need to care.
---
 .../org/scijava/ui/AbstractUserInterface.java | 73 ++++++++++++-------
 .../java/org/scijava/ui/DefaultUIService.java | 23 +-----
 2 files changed, 49 insertions(+), 47 deletions(-)

diff --git a/src/main/java/org/scijava/ui/AbstractUserInterface.java b/src/main/java/org/scijava/ui/AbstractUserInterface.java
index a3894158b..74fb56793 100644
--- a/src/main/java/org/scijava/ui/AbstractUserInterface.java
+++ b/src/main/java/org/scijava/ui/AbstractUserInterface.java
@@ -29,6 +29,7 @@
 
 package org.scijava.ui;
 
+import java.lang.reflect.InvocationTargetException;
 import java.util.List;
 
 import org.scijava.display.Display;
@@ -79,9 +80,11 @@ public abstract class AbstractUserInterface extends AbstractRichPlugin
 
 	@Override
 	public void show() {
-		if (visible) return;
-		createUI();
-		visible = true;
+		runOnDispatchThreadIfNeeded(() -> {
+			if (visible) return;
+			createUI();
+			visible = true;
+		});
 	}
 
 	@Override
@@ -112,32 +115,29 @@ public void show(final Display<?> display) {
 			return;
 		}
 
-		final List<PluginInfo<DisplayViewer<?>>> viewers =
-			uiService.getViewerPlugins();
-
-		DisplayViewer<?> displayViewer = null;
-		for (final PluginInfo<DisplayViewer<?>> info : viewers) {
-			// check that viewer can actually handle the given display
-			final DisplayViewer<?> viewer = pluginService.createInstance(info);
-			if (viewer == null) continue;
-			if (!viewer.canView(display)) continue;
-			if (!viewer.isCompatible(this)) continue;
-			displayViewer = viewer;
-			break; // found a suitable viewer; we are done
-		}
-		if (displayViewer == null) {
-			log.warn("For UI '" + getClass().getName() +
-				"': no suitable viewer for display: " + display);
-			return;
-		}
-
-		final DisplayViewer<?> finalViewer = displayViewer;
-		threadService.queue(new Runnable() {
-			@Override
-			public void run() {
-				uiService.addDisplayViewer(finalViewer);
-				finalViewer.view(AbstractUserInterface.this, display);
+		runOnDispatchThreadIfNeeded(() -> {
+			final List<PluginInfo<DisplayViewer<?>>> viewers =
+				uiService.getViewerPlugins();
+
+			DisplayViewer<?> displayViewer = null;
+			for (final PluginInfo<DisplayViewer<?>> info : viewers) {
+				// check that viewer can actually handle the given display
+				final DisplayViewer<?> viewer = pluginService.createInstance(info);
+				if (viewer == null) continue;
+				if (!viewer.canView(display)) continue;
+				if (!viewer.isCompatible(this)) continue;
+				displayViewer = viewer;
+				break; // found a suitable viewer; we are done
+			}
+			if (displayViewer == null) {
+				log.warn("For UI '" + getClass().getName() +
+					"': no suitable viewer for display: " + display);
+				return;
 			}
+
+			final DisplayViewer<?> finalViewer = displayViewer;
+			uiService.addDisplayViewer(finalViewer);
+			finalViewer.view(AbstractUserInterface.this, display);
 		});
 	}
 
@@ -170,4 +170,21 @@ public void restoreLocation() {
 	protected void createUI() {
 		restoreLocation();
 	}
+
+	// -- Helper methods --
+
+	private void runOnDispatchThreadIfNeeded(Runnable r) {
+		if (requiresEDT()) {
+			try {
+				// Note: Blocks until execution is complete!
+				threadService.invoke(r);
+			}
+			catch (InterruptedException | InvocationTargetException e) {
+				throw new RuntimeException(e);
+			}
+		}
+		else {
+			r.run();
+		}
+	}
 }
diff --git a/src/main/java/org/scijava/ui/DefaultUIService.java b/src/main/java/org/scijava/ui/DefaultUIService.java
index ff5663f91..c4b1e5bac 100644
--- a/src/main/java/org/scijava/ui/DefaultUIService.java
+++ b/src/main/java/org/scijava/ui/DefaultUIService.java
@@ -162,25 +162,10 @@ public void showUI(final String name) {
 	@Override
 	public void showUI(final UserInterface ui) {
 		log.debug("Launching user interface: " + ui.getClass().getName());
-		Runnable showUI = () -> {
-			ui.show();
-			// NB: Also show all the current displays.
-			for (final Display<?> display : displayService.getDisplays()) {
-				ui.show(display);
-			}
-		};
-
-		// Dispatch on EDT if necessary
-		if (ui.requiresEDT()) {
-			try {
-				threadService.invoke(showUI);
-			}
-			catch (InterruptedException | InvocationTargetException e) {
-				throw new RuntimeException(e);
-			}
-		}
-		else {
-			showUI.run();
+		ui.show();
+		// NB: Also show all the current displays.
+		for (final Display<?> display : displayService.getDisplays()) {
+			ui.show(display);
 		}
 		eventService.publish(new UIShownEvent(ui));
 	}