Ticket #11689: 0003-Changed-locks-to-use-java-synchronized.patch

File 0003-Changed-locks-to-use-java-synchronized.patch, 18.2 KB (added by michael2402, 9 years ago)

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

    From eabc47a6687829f175a9212d7a18aeba64c3c835 Mon Sep 17 00:00:00 2001
From: Michael Zangl <michael.zangl@student.kit.edu>
Date: Thu, 6 Aug 2015 14:26:36 +0200
Subject: [PATCH 3/3] Changed locks to use java synchronized.

---
 src/org/openstreetmap/josm/gui/MapView.java | 182 ++++++++++------------------
 1 file changed, 66 insertions(+), 116 deletions(-)

diff --git a/src/org/openstreetmap/josm/gui/MapView.java b/src/org/openstreetmap/josm/gui/MapView.java
index c1ec650..7f114b6 100644
    a b import java.util.List;  
    3131import java.util.ListIterator;
    3232import java.util.Set;
    3333import java.util.concurrent.CopyOnWriteArrayList;
    34 import java.util.concurrent.locks.ReentrantReadWriteLock;
    3534
    3635import javax.swing.AbstractButton;
    3736import javax.swing.ActionMap;
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    229228    }
    230229
    231230    /**
    232      * This is a simple invariant check that tests if the {@link #layerLock} is not write locked.
     231     * This is a simple invariant check that tests if the {@link #layersChangingMutex} is held and the {@link #layersMutex} not.
    233232     * This should be the case whenever a layer listener is invoked.
    234233     */
    235234    private void checkLayerLockNotHeld() {
    236         if (layerLock.isWriteLockedByCurrentThread()) {
    237             Main.warn("layerLock is write-held while a listener was called.");
     235        if (Thread.holdsLock(layersMutex)) {
     236            Main.warn("layersMutex is held while a listener was called.");
     237        }
     238        if (!Thread.holdsLock(layersChangingMutex)) {
     239            Main.warn("layersChangingMutex is not held while a listener was called.");
    238240        }
    239241    }
    240242
    241243    /**
    242      * A list of all layers currently loaded. Locked by {@link #layerLock}.
     244     * A list of all layers currently loaded. Locked by {@link #layersMutex}.
    243245     */
    244246    private final transient List<Layer> layers = new ArrayList<>();
    245247
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    247249     * This lock manages concurrent access to {@link #layers},
    248250     * {@link #editLayer} and {@link #activeLayer}.
    249251     * <p>
    250      * The read lock is always held while those fields are read or while layer change listeners are fired.
     252     * This mutex should never be held when calling "out" (to other classes, listeners, ...)
     253     */
     254    private final transient Object layersMutex = new Object();
     255
     256    /**
     257     * This is a mutex that only locks all methods that change the layers. It is locked in a way that there may be no more changes to layers in other threads while the listeners of the current change are called. Reads are possible in all threads during this phase. This is necessary because many listeners use {@link GuiHelper#runInEDTAndWait(Runnable)}
    251258     */
    252     private final ReentrantReadWriteLock layerLock = new ReentrantReadWriteLock();
     259    private final transient Object layersChangingMutex = new Object();
    253260
    254261    /**
    255262     * The play head marker: there is only one of these so it isn't in any specific layer
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    257264    public transient PlayHeadMarker playHeadMarker = null;
    258265
    259266    /**
    260      * The layer from the layers list that is currently active. Locked by {@link #layerLock}.
     267     * The layer from the layers list that is currently active. Locked by {@link #layersMutex}.
    261268     */
    262269    private transient Layer activeLayer;
    263270
    264271    /**
    265      * The edit layer is the current active data layer. Locked by {@link #layerLock}.
     272     * The edit layer is the current active data layer. Locked by {@link #layersMutex}.
    266273     */
    267274    private transient OsmDataLayer editLayer;
    268275
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    375382     * @param layer the GPX layer
    376383     */
    377384    protected void addGpxLayer(GpxLayer layer) {
    378         layerLock.writeLock().lock();
    379         try {
     385        synchronized (layersMutex) {
    380386            if (layers.isEmpty()) {
    381387                layers.add(layer);
    382388                return;
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    392398                }
    393399            }
    394400            layers.add(0, layer);
    395         } finally {
    396             layerLock.writeLock().unlock();
    397401        }
    398402    }
    399403
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    404408     */
    405409    public void addLayer(Layer layer) {
    406410        boolean isOsmDataLayer = layer instanceof OsmDataLayer;
    407         layerLock.writeLock().lock();
    408         layerLock.readLock().lock();
    409411        EnumSet<LayerListenerType> listenersToFire = EnumSet.noneOf(LayerListenerType.class);
    410         Layer oldActiveLayer = activeLayer;
    411         OsmDataLayer oldEditLayer = editLayer;
    412         try {
    413             try {
     412        synchronized (layersChangingMutex) {
     413            Layer oldActiveLayer;
     414            OsmDataLayer oldEditLayer;
     415            synchronized (layersMutex) {
     416                oldActiveLayer = activeLayer;
     417                oldEditLayer = editLayer;
    414418                if (layer instanceof MarkerLayer && playHeadMarker == null) {
    415419                    playHeadMarker = PlayHeadMarker.create();
    416420                }
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    435439                    // autoselect the new layer
    436440                    listenersToFire.addAll(setActiveLayer(layer, true));
    437441                }
    438             } finally {
    439                 layerLock.writeLock().unlock();
    440442            }
    441443
    442444            fireLayerAdded(layer);
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    447449            layer.addPropertyChangeListener(this);
    448450            Main.addProjectionChangeListener(layer);
    449451            AudioPlayer.reset();
    450         } finally {
    451             layerLock.readLock().unlock();
    452452        }
    453453        if (!listenersToFire.isEmpty()) {
    454454            repaint();
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    457457
    458458    @Override
    459459    protected DataSet getCurrentDataSet() {
    460         layerLock.readLock().lock();
    461         try {
     460        synchronized (layersMutex) {
    462461            if (editLayer != null)
    463462                return editLayer.data;
    464463            else
    465464                return null;
    466         } finally {
    467             layerLock.readLock().unlock();
    468465        }
    469466    }
    470467
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    474471     * @return true if the active data layer (edit layer) is drawable, false otherwise
    475472     */
    476473    public boolean isActiveLayerDrawable() {
    477         layerLock.readLock().lock();
    478         try {
     474        synchronized (layersMutex) {
    479475            return editLayer != null;
    480         } finally {
    481             layerLock.readLock().unlock();
    482476        }
    483477    }
    484478
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    488482     * @return true if the active data layer (edit layer) is visible, false otherwise
    489483     */
    490484    public boolean isActiveLayerVisible() {
    491         layerLock.readLock().lock();
    492         try {
     485        synchronized (layersMutex) {
    493486            return isActiveLayerDrawable() && editLayer.isVisible();
    494         } finally {
    495             layerLock.readLock().unlock();
    496487        }
    497488    }
    498489
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    529520     * @param layer The layer to remove
    530521     */
    531522    public void removeLayer(Layer layer) {
    532         layerLock.writeLock().lock();
    533         layerLock.readLock().lock();
    534 
    535523        EnumSet<LayerListenerType> listenersToFire = EnumSet.noneOf(LayerListenerType.class);
    536         Layer oldActiveLayer = activeLayer;
    537         OsmDataLayer oldEditLayer = editLayer;
    538         try {
    539             try {
     524
     525        synchronized (layersChangingMutex) {
     526            Layer oldActiveLayer;
     527            OsmDataLayer oldEditLayer;
     528            synchronized (layersMutex) {
     529                oldActiveLayer = activeLayer;
     530                oldEditLayer = editLayer;
    540531                List<Layer> layersList = new ArrayList<>(layers);
    541532
    542533                if (!layersList.remove(layer))
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    555546                layers.remove(layer);
    556547                Main.removeProjectionChangeListener(layer);
    557548
    558             } finally {
    559                 layerLock.writeLock().unlock();
    560549            }
    561550            onActiveEditLayerChanged(oldActiveLayer, oldEditLayer, listenersToFire);
    562551            fireLayerRemoved(layer);
    563552            layer.removePropertyChangeListener(this);
    564553            layer.destroy();
    565554            AudioPlayer.reset();
    566         } finally {
    567             layerLock.readLock().unlock();
    568555        }
    569556        repaint();
    570557    }
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    600587     * @param pos       The new position of the layer
    601588     */
    602589    public void moveLayer(Layer layer, int pos) {
    603         layerLock.writeLock().lock();
    604         layerLock.readLock().lock();
    605         EnumSet<LayerListenerType> listenersToFire;
    606         Layer oldActiveLayer = activeLayer;
    607         OsmDataLayer oldEditLayer = editLayer;
    608         try {
    609             try {
     590        synchronized (layersChangingMutex) {
     591            EnumSet<LayerListenerType> listenersToFire;
     592            Layer oldActiveLayer;
     593            OsmDataLayer oldEditLayer;
     594            synchronized (layersMutex) {
     595                oldActiveLayer = activeLayer;
     596                oldEditLayer = editLayer;
    610597                int curLayerPos = layers.indexOf(layer);
    611598                if (curLayerPos == -1)
    612599                    throw new IllegalArgumentException(tr("Layer not in list."));
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    619606                    layers.add(pos, layer);
    620607                }
    621608                listenersToFire = setEditLayer(layers);
    622             } finally {
    623                 layerLock.writeLock().unlock();
    624609            }
    625610            onActiveEditLayerChanged(oldActiveLayer, oldEditLayer, listenersToFire);
    626611            AudioPlayer.reset();
    627         } finally {
    628             layerLock.readLock().unlock();
    629612        }
    630613        repaint();
    631614    }
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    638621     */
    639622    public int getLayerPos(Layer layer) {
    640623        int curLayerPos;
    641         layerLock.readLock().lock();
    642         try {
     624        synchronized (layersMutex) {
    643625            curLayerPos = layers.indexOf(layer);
    644         } finally {
    645             layerLock.readLock().unlock();
    646626        }
    647627        if (curLayerPos == -1)
    648628            throw new IllegalArgumentException(tr("Layer not in list."));
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    659639     * first, layer with the highest Z-Order last.
    660640     */
    661641    public List<Layer> getVisibleLayersInZOrder() {
    662         layerLock.readLock().lock();
    663         try {
    664             List<Layer> ret = new ArrayList<>();
     642        List<Layer> ret = new ArrayList<>();
     643        synchronized (layersMutex) {
    665644            // This is set while we delay the addition of the active layer.
    666645            boolean activeLayerDelayed = false;
    667646            for (ListIterator<Layer> iterator = layers.listIterator(layers.size()); iterator.hasPrevious();) {
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    685664                ret.add(activeLayer);
    686665            }
    687666            return ret;
    688         } finally {
    689             layerLock.readLock().unlock();
    690667        }
    691668    }
    692669
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    875852     * @return An unmodifiable collection of all layers
    876853     */
    877854    public Collection<Layer> getAllLayers() {
    878         layerLock.readLock().lock();
    879         try {
     855        synchronized (layersMutex) {
    880856            return Collections.unmodifiableCollection(new ArrayList<>(layers));
    881         } finally {
    882             layerLock.readLock().unlock();
    883857        }
    884858    }
    885859
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    887861     * @return An unmodifiable ordered list of all layers
    888862     */
    889863    public List<Layer> getAllLayersAsList() {
    890         layerLock.readLock().lock();
    891         try {
     864        synchronized (layersMutex) {
    892865            return Collections.unmodifiableList(new ArrayList<>(layers));
    893         } finally {
    894             layerLock.readLock().unlock();
    895866        }
    896867    }
    897868
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    903874     *     List&lt;WMSLayer&gt; wmsLayers = getLayersOfType(WMSLayer.class);
    904875     * </pre>
    905876     *
    906      * @param ofType The layer type.
    907877     * @return an unmodifiable list of layers of a certain type.
    908878     */
    909879    public <T extends Layer> List<T> getLayersOfType(Class<T> ofType) {
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    916886     * @return the number of layers managed by this map view
    917887     */
    918888    public int getNumLayers() {
    919         layerLock.readLock().lock();
    920         try {
     889        synchronized (layersMutex) {
    921890            return layers.size();
    922         } finally {
    923             layerLock.readLock().unlock();
    924891        }
    925892    }
    926893
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    936903    /**
    937904     * Sets the active edit layer.
    938905     * <p>
    939      * You must own a write {@link #layerLock} when calling this method.
     906     * You must own {@link #layersMutex} when calling this method.
    940907     * @param layersList A list to select that layer from.
    941908     * @return A list of change listeners that should be fired using {@link #onActiveEditLayerChanged(Layer, OsmDataLayer, EnumSet)}
    942909     */
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    946913        // Set new edit layer
    947914        if (newEditLayer != editLayer) {
    948915            if (newEditLayer == null) {
    949                 // Note: Unsafe to call while layer write lock is held.
     916                // Note: Unsafe to call while layersMutex is held.
    950917                getCurrentDataSet().setSelected();
    951918            }
    952919
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    984951     * @throws IllegalArgumentException if layer is not in the lis of layers
    985952     */
    986953    public void setActiveLayer(Layer layer) {
    987         layerLock.writeLock().lock();
    988         layerLock.readLock().lock();
    989         EnumSet<LayerListenerType> listenersToFire;
    990         Layer oldActiveLayer = activeLayer;
    991         OsmDataLayer oldEditLayer = editLayer;
    992         try {
    993             try {
     954        synchronized (layersChangingMutex) {
     955            EnumSet<LayerListenerType> listenersToFire;
     956            Layer oldActiveLayer;
     957            OsmDataLayer oldEditLayer;
     958            synchronized (layersMutex) {
     959                oldActiveLayer = activeLayer;
     960                oldEditLayer = editLayer;
    994961                listenersToFire = setActiveLayer(layer, true);
    995             } finally {
    996                 layerLock.writeLock().unlock();
    997962            }
    998963            onActiveEditLayerChanged(oldActiveLayer, oldEditLayer, listenersToFire);
    999         } finally {
    1000             layerLock.readLock().unlock();
    1001964        }
    1002965        repaint();
    1003966    }
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    1030993     * @return the currently active layer (may be null)
    1031994     */
    1032995    public Layer getActiveLayer() {
    1033         layerLock.readLock().lock();
    1034         try {
     996        synchronized (layersMutex) {
    1035997            return activeLayer;
    1036         } finally {
    1037             layerLock.readLock().unlock();
    1038998        }
    1039999    }
    10401000
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    10891049     * @return the current edit layer. May be null.
    10901050     */
    10911051    public OsmDataLayer getEditLayer() {
    1092         layerLock.readLock().lock();
    1093         try {
     1052        synchronized (layersMutex) {
    10941053            return editLayer;
    1095         } finally {
    1096             layerLock.readLock().unlock();
    10971054        }
    10981055    }
    10991056
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    11041061     * @return true if the list of layers managed by this map view contain layer
    11051062     */
    11061063    public boolean hasLayer(Layer layer) {
    1107         layerLock.readLock().lock();
    1108         try {
     1064        synchronized (layersMutex) {
    11091065            return layers.contains(layer);
    1110         } finally {
    1111             layerLock.readLock().unlock();
    11121066        }
    11131067    }
    11141068
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    11731127     */
    11741128    protected void refreshTitle() {
    11751129        if (Main.parent != null) {
    1176             layerLock.readLock().lock();
    1177             try {
     1130            synchronized (layersMutex) {
    11781131                boolean dirty = editLayer != null &&
    11791132                        (editLayer.requiresSaveToFile() || (editLayer.requiresUploadToServer() && !editLayer.isUploadDiscouraged()));
    11801133                ((JFrame) Main.parent).setTitle((dirty ? "* " : "") + tr("Java OpenStreetMap Editor"));
    11811134                ((JFrame) Main.parent).getRootPane().putClientProperty("Window.documentModified", dirty);
    1182             } finally {
    1183                 layerLock.readLock().unlock();
    11841135            }
    11851136        }
    11861137    }
    … … implements PropertyChangeListener, PreferenceChangedListener, OsmDataLayer.Layer  
    12061157        if (mapMover != null) {
    12071158            mapMover.destroy();
    12081159        }
    1209         layerLock.writeLock().lock();
    1210         try {
    1211             activeLayer = null;
    1212             changedLayer = null;
    1213             editLayer = null;
    1214             layers.clear();
    1215         } finally {
    1216             layerLock.writeLock().unlock();
     1160        synchronized (layersChangingMutex) {
     1161            synchronized (layersMutex) {
     1162                activeLayer = null;
     1163                changedLayer = null;
     1164                editLayer = null;
     1165                layers.clear();
     1166            }
    12171167        }
    12181168        nonChangedLayers.clear();
    12191169        synchronized (temporaryLayers) {