RejectedExecutionException when shutting down JOSM

[Bjoeni]

The following error occurred when shutting down JOSM (main instance) with a couple of layers opened (including Geoimage layers).

I set the component to Core because I think the issue is not the GeoImageLayer trying to call the MainApplication.worker, but that the worker is already shutdown at this point. But I haven't debugged it any further yet.

Possibly somewhat related: #20871

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-06-02 22:03:39 +0200 (Wed, 02 Jun 2021)
Revision:17919
Build-Date:2021-06-02 20:11:30
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (17919 en) Windows 10 64-Bit
OS Build number: Windows 10 Home 2009 (21390)
Memory Usage: 1611 MB / 4028 MB (753 MB allocated, but free)
Java version: 11.0.10+9, AdoptOpenJDK, OpenJDK 64-Bit Server VM
Look and Feel: com.formdev.flatlaf.FlatDarkLaf
Screen: \Display0 1920×1080 (scaling 1.00×1.00) \Display1 2560×1440 (scaling 1.00×1.00)
Maximum Screen Size: 2560×1440
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: Cp1252
System property sun.jnu.encoding: Cp1252
Locale info: en_DE
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Dicedtea-web.bin.location=C:\Program Files\OpenWebStart\javaws, --add-modules=java.scripting,java.sql, --add-exports=java.desktop/com.apple.eawt=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=javafx.graphics/com.sun.javafx.application=ALL-UNNAMED, --add-exports=jdk.deploy/com.sun.deploy.config=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, -Djava.util.Arrays.useLegacyMergeSort=true, --add-exports=java.base/sun.net.www.protocol.jar=ALL-UNNAMED,java.desktop, --add-exports=java.base/jdk.internal.util.jar=ALL-UNNAMED,java.desktop, --add-exports=java.base/com.sun.net.ssl.internal.ssl=ALL-UNNAMED,java.desktop, --add-reads=java.naming=ALL-UNNAMED,java.desktop, --add-exports=java.desktop/sun.awt.X11=ALL-UNNAMED,java.desktop, --add-exports=java.desktop/sun.applet=ALL-UNNAMED,java.desktop,jdk.jsobject, --add-exports=java.base/sun.security.action=ALL-UNNAMED,java.desktop, --add-reads=java.base=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.net.www.protocol.http=ALL-UNNAMED,java.desktop, --add-exports=java.naming/com.sun.jndi.toolkit.url=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.util=ALL-UNNAMED,java.desktop,ALL-UNNAMED, --add-reads=java.desktop=ALL-UNNAMED,java.naming, --add-exports=java.desktop/sun.awt=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.x509=ALL-UNNAMED,java.desktop,ALL-UNNAMED, --add-exports=java.desktop/javax.jnlp=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.provider=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.validator=ALL-UNNAMED,java.desktop]

Plugins:
+ BusRouteMaintenance (1619878317)
+ InfoMode (35543)
+ apache-commons (35524)
+ flatlaf (35734)
+ photo_geotagging (35738)
+ photoadjust (35770)

Last errors/warnings:
- 00000.371 W: extended font config - overriding 'filename.Myanmar_Text=mmrtext.ttf' with 'MMRTEXT.TTF'
- 00000.373 W: extended font config - overriding 'filename.Mongolian_Baiti=monbaiti.ttf' with 'MONBAITI.TTF'
- 00016.505 E: Failed to locate image 'segment'
- 00016.509 E: Failed to locate image 'average'
- 00016.512 E: Failed to locate image 'maintenance'
- 29455.402 E: Handled by bug report queue: java.util.concurrent.RejectedExecutionException: Task org.openstreetmap.josm.gui.layer.geoimage.ImageDisplay$LoadImageRunnable@54c22684 rejected from org.openstreetmap.josm.gui.progress.swing.ProgressMonitorExecutor@32160295[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 41]



=== REPORTED CRASH DATA ===
BugReportExceptionHandler#handleException:
No data collected.

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-1 (48) of JOSM
java.util.concurrent.RejectedExecutionException: Task org.openstreetmap.josm.gui.layer.geoimage.ImageDisplay$LoadImageRunnable@54c22684 rejected from org.openstreetmap.josm.gui.progress.swing.ProgressMonitorExecutor@32160295[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 41]
	at java.base/java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2055)
	at java.base/java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:825)
	at java.base/java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1355)
	at org.openstreetmap.josm.gui.progress.swing.ProgressMonitorExecutor.execute(ProgressMonitorExecutor.java:40)
	at org.openstreetmap.josm.gui.layer.geoimage.ImageDisplay.setImage(ImageDisplay.java:611)
	at org.openstreetmap.josm.gui.layer.geoimage.ImageViewerDialog.displayImages(ImageViewerDialog.java:469)
	at org.openstreetmap.josm.gui.layer.geoimage.ImageViewerDialog.selectedImageChanged(ImageViewerDialog.java:628)
	at org.openstreetmap.josm.data.ImageData.lambda$setSelectedImageIndex$4(ImageData.java:264)
	at org.openstreetmap.josm.tools.ListenerList.fireEvent(ListenerList.java:155)
	at org.openstreetmap.josm.data.ImageData.setSelectedImageIndex(ImageData.java:264)
	at org.openstreetmap.josm.data.ImageData.setSelectedImageIndex(ImageData.java:252)
	at org.openstreetmap.josm.data.ImageData.selectFirstImage(ImageData.java:141)
	at org.openstreetmap.josm.gui.layer.geoimage.ImageViewerDialog.showLayer(ImageViewerDialog.java:622)
	at org.openstreetmap.josm.gui.layer.geoimage.ImageViewerDialog.activeOrEditLayerChanged(ImageViewerDialog.java:611)
	at org.openstreetmap.josm.gui.layer.MainLayerManager.fireActiveLayerChange(MainLayerManager.java:271)
	at org.openstreetmap.josm.gui.layer.MainLayerManager.setActiveLayer(MainLayerManager.java:264)
	at org.openstreetmap.josm.gui.layer.MainLayerManager.realRemoveSingleLayer(MainLayerManager.java:304)
	at org.openstreetmap.josm.gui.layer.LayerManager.realRemoveLayer(LayerManager.java:266)
	at org.openstreetmap.josm.gui.layer.LayerManager.lambda$removeLayer$1(LayerManager.java:248)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWaitWithException(GuiHelper.java:246)
	at org.openstreetmap.josm.gui.layer.LayerManager.removeLayer(LayerManager.java:248)
	at org.openstreetmap.josm.gui.layer.LayerManager.realResetState(LayerManager.java:515)
	at org.openstreetmap.josm.gui.layer.MainLayerManager.realResetState(MainLayerManager.java:493)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWaitWithException(GuiHelper.java:246)
	at org.openstreetmap.josm.gui.layer.LayerManager.resetState(LayerManager.java:506)
	at org.openstreetmap.josm.gui.MainTermination.run(MainTermination.java:43)
	at org.openstreetmap.josm.spi.lifecycle.Lifecycle.exitJosm(Lifecycle.java:121)
	at org.openstreetmap.josm.gui.MainApplication.exitJosm(MainApplication.java:520)
	at org.openstreetmap.josm.gui.MainFrame$ExitWindowAdapter.windowClosing(MainFrame.java:198)
	at java.desktop/java.awt.AWTEventMulticaster.windowClosing(AWTEventMulticaster.java:357)
	at java.desktop/java.awt.Window.processWindowEvent(Window.java:2078)
	at java.desktop/javax.swing.JFrame.processWindowEvent(JFrame.java:298)
	at java.desktop/java.awt.Window.processEvent(Window.java:2037)
	at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:5011)
	at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2321)
	at java.desktop/java.awt.Window.dispatchEventImpl(Window.java:2772)
	at java.desktop/java.awt.Component.dispatchEvent(Component.java:4843)
	at com.formdev.flatlaf.ui.FlatTitlePane.close(FlatTitlePane.java:691)
	at com.formdev.flatlaf.ui.FlatTitlePane.lambda$createButtons$3(FlatTitlePane.java:225)
	at java.desktop/javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1967)
	at java.desktop/javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2308)
	at java.desktop/javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:405)
	at java.desktop/javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:262)
	at java.desktop/javax.swing.plaf.basic.BasicButtonListener.mouseReleased(BasicButtonListener.java:279)
	at java.desktop/java.awt.Component.processMouseEvent(Component.java:6635)
	at java.desktop/javax.swing.JComponent.processMouseEvent(JComponent.java:3342)
	at java.desktop/java.awt.Component.processEvent(Component.java:6400)
	at java.desktop/java.awt.Container.processEvent(Container.java:2263)
	at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:5011)
	at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2321)
	at java.desktop/java.awt.Component.dispatchEvent(Component.java:4843)
	at java.desktop/java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4918)
	at java.desktop/java.awt.LightweightDispatcher.processMouseEvent(Container.java:4547)
	at java.desktop/java.awt.LightweightDispatcher.dispatchEvent(Container.java:4488)
	at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2307)
	at java.desktop/java.awt.Window.dispatchEventImpl(Window.java:2772)
	at java.desktop/java.awt.Component.dispatchEvent(Component.java:4843)
	at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:772)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:95)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:743)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742)
	at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

[Bjoeni]

src/org/openstreetmap/josm/gui/layer/MainLayerManager.java

@@ -299 +299 @@
             return new ArrayList<>();
         }
 
-        if (layer == activeLayer || layer == osmDataLayer) {
+        if (!MainApplication.worker.isShutdown() && (layer == activeLayer || layer == osmDataLayer)) {
             Layer nextActive = suggestNextActiveLayer(layer);
             setActiveLayer(nextActive, true);
         }
 
         Collection<Layer> toDelete = super.realRemoveSingleLayer(layer);
-        if (getLayers().isEmpty()) {
+        if (!MainApplication.worker.isShutdown() && getLayers().isEmpty()) {
             LayerAvailabilityEvent e = new LayerAvailabilityEvent(this, false);
             for (LayerAvailabilityListener l : layerAvailabilityListeners) {
                 l.afterLastLayerRemoved(e);

The problem is that the LayerManager attempts to set a new active layer when another (currently active) layer is removed - even when exiting JOSM. This will cause issues, since the MainApplication.worker is already shutdown at that point. It could also be fixed by shutting down the worker after removing all layers, but I think it's anyways pointless to set a new active layer during shutdown.
(The layerRemoving event required e.g. by the AutoSaveTask will still be fired.)

Also, turns out it's not related to #20871.

[Don-vip]

In 17991/josm:

fix #20989 - don't change active layer after layer removal if main worker is shut down (patch by Bjoeni, modified)

[Don-vip]

I've only applied the first part, I think we still need to fire layer availability events even when the worker is shut down.

[Don-vip]

In 18010/josm:

fix #21107 - see #20989 - revert of r17991 - restore previous chain of events when closing JOSM

[Bjoeni]

In fact I may have applied #20989 too fast, this chain of events must be met when JOSM is closed:
realRemoveSingleLayer => setActiveLayer(null, true) => fireActiveLayerChange => activeOrEditLayerChanged => mapMode.exitMode() => SelectAction.exitMode() => keyDetector.remove*

Well I'm aware that this is the chain of events that is currently happening, however I think that at least some of the steps are unnecessary just before closing. I agree on the can of worms though.

So in that case the easiest solution would probably be to shutdown the worker after removing all layers (that still means that a new active layer is set multiple times while closing, which doesn't seem ideal to me but that's just status quo).

Or fixing it in the geoimage layer, basically making sure that all instances using the worker check if it's still alive.

Or overriding that implementation with a method that simply does nothing when called after shutdown.

Plenty ways to go I guess.

[Don-vip]

In 18012/josm:

fix #20989 - don't refresh GeoImageLayer contents when Main worker is being shut down

[Don-vip]

The chain of events is too old and too broad to consider changing its basic nature (too much impacts). Safer to make sure on a case by case that all layers react accordingly. In the case of GeoImageLayer, there is I think no need to perform anything, so I simply ignored the event in this case.