Ticket #12896: patch-layer-manager-track-removed-layers.patch

src/org/openstreetmap/josm/gui/MapView.java

@@ -57 +57 @@
 import org.openstreetmap.josm.gui.layer.ImageryLayer;
 import org.openstreetmap.josm.gui.layer.Layer;
 import org.openstreetmap.josm.gui.layer.LayerManager;
+import org.openstreetmap.josm.gui.layer.LayerManager.CyclicLayerRemoveException;
 import org.openstreetmap.josm.gui.layer.LayerManager.LayerAddEvent;
 import org.openstreetmap.josm.gui.layer.LayerManager.LayerOrderChangeEvent;
 import org.openstreetmap.josm.gui.layer.LayerManager.LayerRemoveEvent;
@@ -659 +660 @@
      */
     @Deprecated
     public void removeLayer(Layer layer) {
-        layerManager.removeLayer(layer);
+        try {
+            layerManager.removeLayer(layer);
+        } catch (CyclicLayerRemoveException e) {
+            // ignore...
+            Main.warn(e);
+        }
     }
 
     @Override

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

@@ -3 +3 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.IdentityHashMap;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.openstreetmap.josm.gui.util.GuiHelper;
@@ -114 +116 @@
         LayerOrderChangeEvent(LayerManager source) {
             super(source);
         }
+    }
+
+    /**
+     * An exception to be thrown when a cyclic remove is detected (remove inside removed listener)
+     * @author Michael Zangl
+     * @since xxx
+     */
+    public static class CyclicLayerRemoveException extends IllegalArgumentException {
+
+        /**
+         * Create a new exception
+         * @param message The message
+         */
+        public CyclicLayerRemoveException(String message) {
+            super(message);
+        }
 
+        @Override
+        public String toString() {
+            return "CyclicLayerRemoveException [" + getMessage() + "]";
+        }
     }
 
     /**
@@ -125 +147 @@
     private final List<LayerChangeListener> layerChangeListeners = new CopyOnWriteArrayList<>();
 
     /**
+     * A set of layers that are currently removed. To detect {@link #removeLayer(Layer)} calls in the layer remove listener.
+     */
+    private final Set<Layer> removingLayers = Collections.newSetFromMap(new IdentityHashMap<Layer, Boolean>());
+
+    /**
      * Add a layer. The layer will be added at a given psoition.
      * @param layer The layer to add
      */
@@ -154 +181 @@
      * Remove the layer from the mapview. If the layer was in the list before,
      * an LayerChange event is fired.
      * @param layer The layer to remove
+     * @throws IllegalArgumentException When the layer is not in this list
+     * @throws CyclicLayerRemoveException When {@link #removeLayer(Layer)} is called for a removed layer inside the layer removed listener.
      */
     public void removeLayer(final Layer layer) {
         // we force this on to the EDT Thread to make events fire from there.
@@ -161 +190 @@
         GuiHelper.runInEDTAndWaitWithException(new Runnable() {
             @Override
             public void run() {
-                realRemoveLayer(layer);
+                checkContainsLayer(layer);
+                if (!removingLayers.add(layer)) {
+                    throw new CyclicLayerRemoveException("Caclic remove for layer " + layer);
+                }
+                try {
+                    realRemoveLayer(layer);
+                } finally {
+                    removingLayers.remove(layer);
+                }
             }
         });
     }
 
     protected synchronized void realRemoveLayer(Layer layer) {
-        checkContainsLayer(layer);
-
         fireLayerRemoving(layer);
         layers.remove(layer);
     }

test/unit/org/openstreetmap/josm/gui/layer/LayerManagerTest.java

@@ -21 +21 @@
 import org.openstreetmap.josm.data.Bounds;
 import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor;
 import org.openstreetmap.josm.gui.MapView;
+import org.openstreetmap.josm.gui.layer.LayerManager.CyclicLayerRemoveException;
 import org.openstreetmap.josm.gui.layer.LayerManager.LayerAddEvent;
 import org.openstreetmap.josm.gui.layer.LayerManager.LayerChangeListener;
 import org.openstreetmap.josm.gui.layer.LayerManager.LayerOrderChangeEvent;
@@ -229 +230 @@
     }
 
     /**
+     * {@link LayerManager#removeLayer(Layer)} fails if a removeLayer of a layer that is removed is called inside the layer remove listener.
+     */
+    @Test(expected = CyclicLayerRemoveException.class)
+    public void testRemoveLayerFailsWhenRemoveCycle() {
+        final AbstractTestLayer layer1 = new AbstractTestLayer();
+        AbstractTestLayer layer2 = new AbstractTestLayer();
+        layerManager.addLayer(layer1);
+        layerManager.addLayer(layer2);
+
+        layerManager.addLayerChangeListener(new LayerChangeListener() {
+            @Override
+            public void layerRemoving(LayerRemoveEvent e) {
+                layerManager.removeLayer(layer1);
+            }
+
+            @Override
+            public void layerOrderChanged(LayerOrderChangeEvent e) {
+            }
+
+            @Override
+            public void layerAdded(LayerAddEvent e) {
+            }
+        });
+        layerManager.removeLayer(layer1);
+    }
+    /**
      * {@link LayerManager#moveLayer(Layer, int)}
      */
     @Test