Index: /trunk/src/org/openstreetmap/josm/gui/MapView.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/MapView.java (revision 8641) +++ /trunk/src/org/openstreetmap/josm/gui/MapView.java (revision 8642) @@ -32,5 +32,4 @@ import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.locks.ReentrantReadWriteLock; import javax.swing.AbstractButton; @@ -202,5 +201,4 @@ */ protected void fireActiveLayerChanged(Layer oldLayer, Layer newLayer) { - checkLayerLockNotHeld(); for (LayerChangeListener l : layerChangeListeners) { l.activeLayerChange(oldLayer, newLayer); @@ -209,5 +207,4 @@ protected void fireLayerAdded(Layer newLayer) { - checkLayerLockNotHeld(); for (MapView.LayerChangeListener l : MapView.layerChangeListeners) { l.layerAdded(newLayer); @@ -216,5 +213,4 @@ protected void fireLayerRemoved(Layer layer) { - checkLayerLockNotHeld(); for (MapView.LayerChangeListener l : MapView.layerChangeListeners) { l.layerRemoved(layer); @@ -223,17 +219,6 @@ protected void fireEditLayerChanged(OsmDataLayer oldLayer, OsmDataLayer newLayer) { - checkLayerLockNotHeld(); for (EditLayerChangeListener l : editLayerChangeListeners) { l.editLayerChanged(oldLayer, newLayer); - } - } - - /** - * This is a simple invariant check that tests if the {@link #layerLock} is not write locked. - * This should be the case whenever a layer listener is invoked. - */ - private void checkLayerLockNotHeld() { - if (layerLock.isWriteLockedByCurrentThread()) { - Main.warn("layerLock is write-held while a listener was called."); } } @@ -250,5 +235,5 @@ * The read lock is always held while those fields are read or while layer change listeners are fired. */ - private final ReentrantReadWriteLock layerLock = new ReentrantReadWriteLock(); + //private final ReentrantReadWriteLock layerLock = new ReentrantReadWriteLock(); /** @@ -376,6 +361,5 @@ */ protected void addGpxLayer(GpxLayer layer) { - layerLock.writeLock().lock(); - try { + synchronized (layers) { if (layers.isEmpty()) { layers.add(layer); @@ -393,6 +377,4 @@ } layers.add(0, layer); - } finally { - layerLock.writeLock().unlock(); } } @@ -409,34 +391,28 @@ OsmDataLayer oldEditLayer = editLayer; - layerLock.writeLock().lock(); - layerLock.readLock().lock(); - try { - try { - if (layer instanceof MarkerLayer && playHeadMarker == null) { - playHeadMarker = PlayHeadMarker.create(); + synchronized (layers) { + if (layer instanceof MarkerLayer && playHeadMarker == null) { + playHeadMarker = PlayHeadMarker.create(); + } + + if (layer instanceof GpxLayer) { + addGpxLayer((GpxLayer) layer); + } else if (layers.isEmpty()) { + layers.add(layer); + } else if (layer.isBackgroundLayer()) { + int i = 0; + for (; i < layers.size(); i++) { + if (layers.get(i).isBackgroundLayer()) { + break; + } } - - if (layer instanceof GpxLayer) { - addGpxLayer((GpxLayer) layer); - } else if (layers.isEmpty()) { - layers.add(layer); - } else if (layer.isBackgroundLayer()) { - int i = 0; - for (; i < layers.size(); i++) { - if (layers.get(i).isBackgroundLayer()) { - break; - } - } - layers.add(i, layer); - } else { - layers.add(0, layer); - } - - if (isOsmDataLayer || oldActiveLayer == null) { - // autoselect the new layer - listenersToFire.addAll(setActiveLayer(layer, true)); - } - } finally { - layerLock.writeLock().unlock(); + layers.add(i, layer); + } else { + layers.add(0, layer); + } + + if (isOsmDataLayer || oldActiveLayer == null) { + // autoselect the new layer + listenersToFire.addAll(setActiveLayer(layer, true)); } @@ -449,6 +425,4 @@ Main.addProjectionChangeListener(layer); AudioPlayer.reset(); - } finally { - layerLock.readLock().unlock(); } if (!listenersToFire.isEmpty()) { @@ -459,12 +433,9 @@ @Override protected DataSet getCurrentDataSet() { - layerLock.readLock().lock(); - try { + synchronized (layers) { if (editLayer != null) return editLayer.data; else return null; - } finally { - layerLock.readLock().unlock(); } } @@ -476,9 +447,6 @@ */ public boolean isActiveLayerDrawable() { - layerLock.readLock().lock(); - try { + synchronized (layers) { return editLayer != null; - } finally { - layerLock.readLock().unlock(); } } @@ -490,9 +458,6 @@ */ public boolean isActiveLayerVisible() { - layerLock.readLock().lock(); - try { + synchronized (layers) { return isActiveLayerDrawable() && editLayer.isVisible(); - } finally { - layerLock.readLock().unlock(); } } @@ -535,29 +500,23 @@ OsmDataLayer oldEditLayer = editLayer; - layerLock.writeLock().lock(); - layerLock.readLock().lock(); - try { - try { - List layersList = new ArrayList<>(layers); - - if (!layersList.remove(layer)) - return; - - listenersToFire = setEditLayer(layersList); - - if (layer == activeLayer) { - listenersToFire.addAll(setActiveLayer(determineNextActiveLayer(layersList), false)); - } - - if (layer instanceof OsmDataLayer) { - ((OsmDataLayer) layer).removeLayerPropertyChangeListener(this); - } - - layers.remove(layer); - Main.removeProjectionChangeListener(layer); - - } finally { - layerLock.writeLock().unlock(); - } + synchronized (layers) { + List layersList = new ArrayList<>(layers); + + if (!layersList.remove(layer)) + return; + + listenersToFire = setEditLayer(layersList); + + if (layer == activeLayer) { + listenersToFire.addAll(setActiveLayer(determineNextActiveLayer(layersList), false)); + } + + if (layer instanceof OsmDataLayer) { + ((OsmDataLayer) layer).removeLayerPropertyChangeListener(this); + } + + layers.remove(layer); + Main.removeProjectionChangeListener(layer); + onActiveEditLayerChanged(oldActiveLayer, oldEditLayer, listenersToFire); fireLayerRemoved(layer); @@ -565,6 +524,4 @@ layer.destroy(); AudioPlayer.reset(); - } finally { - layerLock.readLock().unlock(); } repaint(); @@ -606,27 +563,19 @@ OsmDataLayer oldEditLayer = editLayer; - layerLock.writeLock().lock(); - layerLock.readLock().lock(); - try { - try { - int curLayerPos = layers.indexOf(layer); - if (curLayerPos == -1) - throw new IllegalArgumentException(tr("Layer not in list.")); - if (pos == curLayerPos) - return; // already in place. - layers.remove(curLayerPos); - if (pos >= layers.size()) { - layers.add(layer); - } else { - layers.add(pos, layer); - } - listenersToFire = setEditLayer(layers); - } finally { - layerLock.writeLock().unlock(); - } + synchronized (layers) { + int curLayerPos = layers.indexOf(layer); + if (curLayerPos == -1) + throw new IllegalArgumentException(tr("Layer not in list.")); + if (pos == curLayerPos) + return; // already in place. + layers.remove(curLayerPos); + if (pos >= layers.size()) { + layers.add(layer); + } else { + layers.add(pos, layer); + } + listenersToFire = setEditLayer(layers); onActiveEditLayerChanged(oldActiveLayer, oldEditLayer, listenersToFire); AudioPlayer.reset(); - } finally { - layerLock.readLock().unlock(); } repaint(); @@ -641,9 +590,6 @@ public int getLayerPos(Layer layer) { int curLayerPos; - layerLock.readLock().lock(); - try { + synchronized (layers) { curLayerPos = layers.indexOf(layer); - } finally { - layerLock.readLock().unlock(); } if (curLayerPos == -1) @@ -662,6 +608,5 @@ */ public List getVisibleLayersInZOrder() { - layerLock.readLock().lock(); - try { + synchronized (layers) { List ret = new ArrayList<>(); // This is set while we delay the addition of the active layer. @@ -688,6 +633,4 @@ } return ret; - } finally { - layerLock.readLock().unlock(); } } @@ -878,9 +821,6 @@ */ public Collection getAllLayers() { - layerLock.readLock().lock(); - try { + synchronized (layers) { return Collections.unmodifiableCollection(new ArrayList<>(layers)); - } finally { - layerLock.readLock().unlock(); } } @@ -890,9 +830,6 @@ */ public List getAllLayersAsList() { - layerLock.readLock().lock(); - try { + synchronized (layers) { return Collections.unmodifiableList(new ArrayList<>(layers)); - } finally { - layerLock.readLock().unlock(); } } @@ -919,9 +856,6 @@ */ public int getNumLayers() { - layerLock.readLock().lock(); - try { + synchronized (layers) { return layers.size(); - } finally { - layerLock.readLock().unlock(); } } @@ -987,18 +921,11 @@ */ public void setActiveLayer(Layer layer) { - layerLock.writeLock().lock(); - layerLock.readLock().lock(); EnumSet listenersToFire; - Layer oldActiveLayer = activeLayer; - OsmDataLayer oldEditLayer = editLayer; - try { - try { - listenersToFire = setActiveLayer(layer, true); - } finally { - layerLock.writeLock().unlock(); - } + + synchronized (layers) { + Layer oldActiveLayer = activeLayer; + OsmDataLayer oldEditLayer = editLayer; + listenersToFire = setActiveLayer(layer, true); onActiveEditLayerChanged(oldActiveLayer, oldEditLayer, listenersToFire); - } finally { - layerLock.readLock().unlock(); } repaint(); @@ -1033,9 +960,6 @@ */ public Layer getActiveLayer() { - layerLock.readLock().lock(); - try { + synchronized (layers) { return activeLayer; - } finally { - layerLock.readLock().unlock(); } } @@ -1092,9 +1016,6 @@ */ public OsmDataLayer getEditLayer() { - layerLock.readLock().lock(); - try { + synchronized (layers) { return editLayer; - } finally { - layerLock.readLock().unlock(); } } @@ -1107,9 +1028,6 @@ */ public boolean hasLayer(Layer layer) { - layerLock.readLock().lock(); - try { + synchronized (layers) { return layers.contains(layer); - } finally { - layerLock.readLock().unlock(); } } @@ -1176,12 +1094,9 @@ protected void refreshTitle() { if (Main.parent != null) { - layerLock.readLock().lock(); - try { + synchronized (layers) { boolean dirty = editLayer != null && (editLayer.requiresSaveToFile() || (editLayer.requiresUploadToServer() && !editLayer.isUploadDiscouraged())); ((JFrame) Main.parent).setTitle((dirty ? "* " : "") + tr("Java OpenStreetMap Editor")); ((JFrame) Main.parent).getRootPane().putClientProperty("Window.documentModified", dirty); - } finally { - layerLock.readLock().unlock(); } } @@ -1209,14 +1124,11 @@ mapMover.destroy(); } - layerLock.writeLock().lock(); - try { + synchronized (layers) { activeLayer = null; changedLayer = null; editLayer = null; layers.clear(); - } finally { - layerLock.writeLock().unlock(); - } - nonChangedLayers.clear(); + nonChangedLayers.clear(); + } synchronized (temporaryLayers) { temporaryLayers.clear();