Changeset 17143 in josm

Timestamp:

2020-10-10T17:18:25+02:00 (4 years ago)

Author:

GerdP

Message:

see #19885: memory leak with "temporary" objects in validator and actions

• use ChangeNodesCommand instead of ChangeCommand
• further simplifications

This one was a bit more complex. I hope I didn't mess up any special use case.

File:

• trunk/src/org/openstreetmap/josm/actions/mapmode/DrawAction.java (modified) (10 diffs)

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

@@ -33 +33 @@
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.command.AddCommand;
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.SequenceCommand;
@@ -516 +516 @@
         Collection<Command> cmds = new LinkedList<>();
         Collection<OsmPrimitive> newSelection = new LinkedList<>(ds.getSelected());
-        List<Way> reuseWays = new ArrayList<>();
-        List<Way> replacedWays = new ArrayList<>();
+        Map<Way, List<Node>> reuseWays = new HashMap<>();
 
         if (newNode) {
@@ -538 +537 @@
                     tryToMoveNodeOnIntersection(wss, n);
                 }
-                insertNodeIntoAllNearbySegments(wss, n, newSelection, cmds, replacedWays, reuseWays);
+                insertNodeIntoAllNearbySegments(wss, n, newSelection, cmds, reuseWays);
             }
         }
@@ -600 +599 @@
                 // user wants a new way.
                 Way way = alt ? null : (selectedWay != null ? selectedWay : getWayForNode(n0));
-                Way wayToSelect;
 
                 // Don't allow creation of self-overlapping ways
@@ -613 +611 @@
                     way = new Way();
                     way.addNode(n0);
+                    way.addNode(n);
                     cmds.add(new AddCommand(ds, way));
-                    wayToSelect = way;
                 } else {
-                    int i;
-                    if ((i = replacedWays.indexOf(way)) != -1) {
-                        way = reuseWays.get(i);
-                        wayToSelect = way;
-                    } else {
-                        wayToSelect = way;
-                        Way wnew = new Way(way);
-                        cmds.add(new ChangeCommand(way, wnew));
-                        way = wnew;
+                    List<Node> modNodes = reuseWays.get(way);
+                    boolean reuse = (modNodes != null);
+                    if (modNodes == null) {
+                        modNodes = way.getNodes();
+                    }
+                    // Connected to a node that's already in the way
+                    if (modNodes.contains(n)) {
+                        wayIsFinished = true;
+                        selection.clear();
+                    }
+                    if (modNodes.get(modNodes.size() - 1) == n0)
+                        modNodes.add(n);
+                    else
+                        modNodes.add(0, n);
+                    if (!reuse) {
+                        cmds.add(new ChangeNodesCommand(way, modNodes));
                     }
                 }
 
-                // Connected to a node that's already in the way
-                if (way.containsNode(n)) {
-                    wayIsFinished = true;
-                    selection.clear();
-                }
-
-                // Add new node to way
-                if (way.getNode(way.getNodesCount() - 1) == n0) {
-                    way.addNode(n);
-                } else {
-                    way.addNode(0, n);
-                }
-
                 extendedWay = true;
                 newSelection.clear();
-                newSelection.add(wayToSelect);
+                newSelection.add(way);
             }
         }
@@ -670 +662 @@
     }
 
-    private static String getTitle(boolean newNode, Node n, Collection<OsmPrimitive> newSelection, List<Way> reuseWays,
+    private static String getTitle(boolean newNode, Node n, Collection<OsmPrimitive> newSelection, Map<Way, List<Node>> reuseWays,
             boolean extendedWay) {
         String title;
@@ -678 +670 @@
             } else {
                 title = tr("Add node into way");
-                for (Way w : reuseWays) {
+                for (Way w : reuseWays.keySet()) {
                     newSelection.remove(w);
                 }
@@ -695 +687 @@
 
     private void insertNodeIntoAllNearbySegments(List<WaySegment> wss, Node n, Collection<OsmPrimitive> newSelection,
-            Collection<Command> cmds, List<Way> replacedWays, List<Way> reuseWays) {
+            Collection<Command> cmds, Map<Way, List<Node>> reuseWays) {
         Map<Way, List<Integer>> insertPoints = new HashMap<>();
         for (WaySegment ws : wss) {
@@ -715 +707 @@
             List<Integer> is = insertPoint.getValue();
 
-            Way wnew = new Way(w);
-
+            List<Node> modNodes = w.getNodes();
             pruneSuccsAndReverse(is);
             for (int i : is) {
                 segSet.add(Pair.sort(new Pair<>(w.getNode(i), w.getNode(i+1))));
-                wnew.addNode(i + 1, n);
+                modNodes.add(i + 1, n);
             }
 
@@ -732 +723 @@
             }
 
-            cmds.add(new ChangeCommand(insertPoint.getKey(), wnew));
-            replacedWays.add(insertPoint.getKey());
-            reuseWays.add(wnew);
+            cmds.add(new ChangeNodesCommand(insertPoint.getKey(), modNodes));
+            reuseWays.put(w, modNodes);
         }