Changeset 12065 in josm

Timestamp:

2017-05-05T16:27:43+02:00 (7 years ago)

Author:

michael2402

Message:

More ds.beginUpdate()/endUpdate() try/finally fixes, checked all calls of beginUpdate().

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/mapmode/SelectAction.java (modified) (3 diffs)
• data/osm/DataSet.java (modified) (1 diff)
• gui/MapStatus.java (modified) (1 diff)
• io/AbstractReader.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/actions/mapmode/SelectAction.java

@@ -715 +715 @@
             if (startEN == null) return false; // fix #8128
             ds.beginUpdate();
-            if (c instanceof MoveCommand && affectedNodes.equals(((MoveCommand) c).getParticipatingPrimitives())) {
-                ((MoveCommand) c).saveCheckpoint();
-                ((MoveCommand) c).applyVectorTo(currentEN);
-            } else {
-                c = new MoveCommand(selection, startEN, currentEN);
-                Main.main.undoRedo.add(c);
-            }
-            for (Node n : affectedNodes) {
-                LatLon ll = n.getCoor();
-                if (ll != null && ll.isOutSideWorld()) {
-                    // Revert move
-                    ((MoveCommand) c).resetToCheckpoint();
-                    ds.endUpdate();
-                    JOptionPane.showMessageDialog(
-                            Main.parent,
-                            tr("Cannot move objects outside of the world."),
-                            tr("Warning"),
-                            JOptionPane.WARNING_MESSAGE);
-                    mv.setNewCursor(cursor, this);
-                    return false;
-                }
+            try {
+                if (c instanceof MoveCommand && affectedNodes.equals(((MoveCommand) c).getParticipatingPrimitives())) {
+                    ((MoveCommand) c).saveCheckpoint();
+                    ((MoveCommand) c).applyVectorTo(currentEN);
+                } else {
+                    c = new MoveCommand(selection, startEN, currentEN);
+                    Main.main.undoRedo.add(c);
+                }
+                for (Node n : affectedNodes) {
+                    LatLon ll = n.getCoor();
+                    if (ll != null && ll.isOutSideWorld()) {
+                        // Revert move
+                        ((MoveCommand) c).resetToCheckpoint();
+                        // TODO: We might use a simple notification in the lower left corner.
+                        JOptionPane.showMessageDialog(
+                                Main.parent,
+                                tr("Cannot move objects outside of the world."),
+                                tr("Warning"),
+                                JOptionPane.WARNING_MESSAGE);
+                        mv.setNewCursor(cursor, this);
+                        return false;
+                    }
+                }
+            } finally {
+                ds.endUpdate();
             }
         } else {
@@ -745 +749 @@
 
             ds.beginUpdate();
-
-            if (mode == Mode.ROTATE) {
-                if (c instanceof RotateCommand && affectedNodes.equals(((RotateCommand) c).getTransformedNodes())) {
-                    ((RotateCommand) c).handleEvent(currentEN);
-                } else {
-                    Main.main.undoRedo.add(new RotateCommand(selection, currentEN));
-                }
-            } else if (mode == Mode.SCALE) {
-                if (c instanceof ScaleCommand && affectedNodes.equals(((ScaleCommand) c).getTransformedNodes())) {
-                    ((ScaleCommand) c).handleEvent(currentEN);
-                } else {
-                    Main.main.undoRedo.add(new ScaleCommand(selection, currentEN));
-                }
-            }
-
-            Collection<Way> ways = ds.getSelectedWays();
-            if (doesImpactStatusLine(affectedNodes, ways)) {
-                Main.map.statusLine.setDist(ways);
-            }
-        }
-        ds.endUpdate();
+            try {
+                if (mode == Mode.ROTATE) {
+                    if (c instanceof RotateCommand && affectedNodes.equals(((RotateCommand) c).getTransformedNodes())) {
+                        ((RotateCommand) c).handleEvent(currentEN);
+                    } else {
+                        Main.main.undoRedo.add(new RotateCommand(selection, currentEN));
+                    }
+                } else if (mode == Mode.SCALE) {
+                    if (c instanceof ScaleCommand && affectedNodes.equals(((ScaleCommand) c).getTransformedNodes())) {
+                        ((ScaleCommand) c).handleEvent(currentEN);
+                    } else {
+                        Main.main.undoRedo.add(new ScaleCommand(selection, currentEN));
+                    }
+                }
+
+                Collection<Way> ways = ds.getSelectedWays();
+                if (doesImpactStatusLine(affectedNodes, ways)) {
+                    Main.map.statusLine.setDist(ways);
+                }
+            } finally {
+                ds.endUpdate();
+            }
+        }
         return true;
     }
@@ -907 +913 @@
             Command c = getLastCommandInDataset(ds);
             ds.beginUpdate();
-            if (c instanceof MoveCommand && affectedNodes.equals(((MoveCommand) c).getParticipatingPrimitives())) {
-                Node selectedNode = selNodes.iterator().next();
-                EastNorth selectedEN = selectedNode.getEastNorth();
-                EastNorth targetEN = target.getEastNorth();
-                ((MoveCommand) c).moveAgain(targetEN.getX() - selectedEN.getX(),
-                                            targetEN.getY() - selectedEN.getY());
-            }
-            ds.endUpdate();
+            try {
+                if (c instanceof MoveCommand && affectedNodes.equals(((MoveCommand) c).getParticipatingPrimitives())) {
+                    Node selectedNode = selNodes.iterator().next();
+                    EastNorth selectedEN = selectedNode.getEastNorth();
+                    EastNorth targetEN = target.getEastNorth();
+                    ((MoveCommand) c).moveAgain(targetEN.getX() - selectedEN.getX(),
+                                                targetEN.getY() - selectedEN.getY());
+                }
+            } finally {
+                ds.endUpdate();
+            }
         }
 

trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -1228 +1228 @@
     public void invalidateEastNorthCache() {
         if (Main.getProjection() == null) return; // sanity check
+        beginUpdate();
         try {
-            beginUpdate();
             for (Node n: getNodes()) {
                 n.invalidateEastNorthCache();

trunk/src/org/openstreetmap/josm/gui/MapStatus.java

@@ -293 +293 @@
                 boolean isAtOldPosition = mouseNotMoved && popup != null;
                 boolean middleMouseDown = (ms.modifiers & MouseEvent.BUTTON2_DOWN_MASK) != 0;
+
+                ds = mv.getLayerManager().getEditDataSet();
+                if (ds != null) {
+                    // This is not perfect, if current dataset was changed during execution, the lock would be useless
+                    if (isAtOldPosition && middleMouseDown) {
+                        // Write lock is necessary when selecting in popupCycleSelection
+                        // locks can not be upgraded -> if do read lock here and write lock later
+                        // (in OsmPrimitive.updateFlags) then always occurs deadlock (#5814)
+                        ds.beginUpdate();
+                    } else {
+                        ds.getReadLock().lock();
+                    }
+                }
                 try {
-                    ds = mv.getLayerManager().getEditDataSet();
-                    if (ds != null) {
-                        // This is not perfect, if current dataset was changed during execution, the lock would be useless
-                        if (isAtOldPosition && middleMouseDown) {
-                            // Write lock is necessary when selecting in popupCycleSelection
-                            // locks can not be upgraded -> if do read lock here and write lock later
-                            // (in OsmPrimitive.updateFlags) then always occurs deadlock (#5814)
-                            ds.beginUpdate();
-                        } else {
-                            ds.getReadLock().lock();
-                        }
-                    }
-
                     // Set the text label in the bottom status bar
                     // "if mouse moved only" was added to stop heap growing

trunk/src/org/openstreetmap/josm/io/AbstractReader.java

@@ -194 +194 @@
 
     protected final void prepareDataSet() throws IllegalDataException {
+        ds.beginUpdate();
         try {
-            ds.beginUpdate();
             processNodesAfterParsing();
             processWaysAfterParsing();