Changeset 11011 in josm

Timestamp:

2016-09-17T16:44:15+02:00 (8 years ago)

Author:

michael2402

Message:

Change LayerManager#getLayers() to not require a lock any more. Fixes #13644

File:

• trunk/src/org/openstreetmap/josm/gui/layer/LayerManager.java (modified) (9 diffs)

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

@@ -10 +10 @@
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.function.Consumer;
 
 import org.openstreetmap.josm.Main;
@@ -170 +171 @@
 
     /**
-     * This is the list of layers we manage.
-     */
-    private final List<Layer> layers = new ArrayList<>();
+     * This is the list of layers we manage. The list is unmodifyable It is only changed in the EDT.
+     * @see LayerManager#updateLayers(Consumer)
+     */
+    private volatile List<Layer> layers = Collections.emptyList();
 
     private final List<LayerChangeListener> layerChangeListeners = new CopyOnWriteArrayList<>();
@@ -228 +230 @@
 
     protected Collection<Layer> realRemoveSingleLayer(Layer layerToRemove) {
-        layers.remove(layerToRemove);
+        updateLayers(mutableLayers -> mutableLayers.remove(layerToRemove));
         return fireLayerRemoving(layerToRemove);
     }
@@ -248 +250 @@
         checkPosition(position);
 
-        int curLayerPos = layers.indexOf(layer);
+        int curLayerPos = getLayers().indexOf(layer);
         if (position == curLayerPos)
             return; // already in place.
-        layers.remove(curLayerPos);
-        insertLayerAt(layer, position);
+        // update needs to be done in one run
+        updateLayers(mutableLayers -> {
+            mutableLayers.remove(curLayerPos);
+            insertLayerAt(mutableLayers, layer, position);
+        });
         fireLayerOrderChanged();
     }
@@ -262 +267 @@
      */
     private void insertLayerAt(Layer layer, int position) {
+        updateLayers(mutableLayers -> insertLayerAt(mutableLayers, layer, position));
+    }
+
+    private static void insertLayerAt(List<Layer> layers, Layer layer, int position) {
         if (position == layers.size()) {
             layers.add(layer);
@@ -275 +284 @@
      */
     private void checkPosition(int position) {
-        if (position < 0 || position > layers.size()) {
+        if (position < 0 || position > getLayers().size()) {
             throw new IndexOutOfBoundsException("Position " + position + " out of range.");
         }
+    }
+
+    /**
+     * Update the {@link #layers} field. This method should be used instead of a direct field access.
+     * @param mutator A method that gets the writable list of layers and should modify it.
+     */
+    private void updateLayers(Consumer<List<Layer>> mutator) {
+        GuiHelper.assertCallFromEdt();
+        ArrayList<Layer> newLayers = new ArrayList<>(getLayers());
+        mutator.accept(newLayers);
+        layers = Collections.unmodifiableList(newLayers);
     }
 
@@ -284 +304 @@
      * @return The list of layers.
      */
-    public synchronized List<Layer> getLayers() {
-        return Collections.unmodifiableList(new ArrayList<>(layers));
+    public List<Layer> getLayers() {
+        return layers;
     }
 
@@ -299 +319 @@
      * @return an unmodifiable list of layers of a certain type.
      */
-    public synchronized <T extends Layer> List<T> getLayersOfType(Class<T> ofType) {
+    public <T extends Layer> List<T> getLayersOfType(Class<T> ofType) {
         return new ArrayList<>(Utils.filteredCollection(getLayers(), ofType));
     }
@@ -309 +329 @@
      * @return true if the list of layers managed by this map view contain layer
      */
-    public synchronized boolean containsLayer(Layer layer) {
-        return layers.contains(layer);
+    public boolean containsLayer(Layer layer) {
+        return getLayers().contains(layer);
     }