Changeset 17205 in josm for trunk

Timestamp:

2020-10-15T15:17:57+02:00 (4 years ago)

Author:

GerdP

Message:

see #19885: memory leak with "temporary" objects in validator and actions
Start to use ChangeMembersCommand instead of ChangeCommand in those places where the cloned relation was only created for the ChangeCommand and not referenced elsewhere.

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/UnGlueAction.java (modified) (4 diffs)
• command/DeleteCommand.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/actions/UnGlueAction.java

@@ -23 +23 @@
 import org.openstreetmap.josm.command.AddCommand;
 import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangeMembersCommand;
 import org.openstreetmap.josm.command.ChangeNodesCommand;
 import org.openstreetmap.josm.command.Command;
@@ -298 +299 @@
      * @param cmds List of commands that will contain the new "add node" command
      * @param newNodes List of nodes that will contain the new node
-     * @return new way The modified way. Change command must be handled by the caller
+     * @return The modified list of way nodes. Change command must be handled by the caller
      */
     private static List<Node> modifyWay(Node originalNode, Way w, List<Command> cmds, List<Node> newNodes) {
@@ -332 +333 @@
                 continue;
             }
-            Relation newRel = new Relation(r);
+            List<RelationMember> newMembers = new ArrayList<>(r.getMembers());
             // loop backwards because we add or remove members, works also when nodes appear
             // multiple times in the same relation
@@ -341 +342 @@
                     continue;
                 for (Node n : newNodes) {
-                    newRel.addMember(i + 1, new RelationMember(rm.getRole(), n));
+                    newMembers.add(i + 1, new RelationMember(rm.getRole(), n));
                 }
                 if (ExistingBothNew.NEW == memberships) {
                     // remove old member
-                    newRel.removeMember(i);
+                    newMembers.remove(i);
                 }
                 changed = true;
             }
             if (changed) {
-                cmds.add(new ChangeCommand(r, newRel));
+                cmds.add(new ChangeMembersCommand(r, newMembers));
             }
         }

trunk/src/org/openstreetmap/josm/command/DeleteCommand.java

@@ -29 +29 @@
 import org.openstreetmap.josm.data.osm.PrimitiveData;
 import org.openstreetmap.josm.data.osm.Relation;
+import org.openstreetmap.josm.data.osm.RelationMember;
 import org.openstreetmap.josm.data.osm.RelationToChildReference;
 import org.openstreetmap.josm.data.osm.Way;
@@ -307 +308 @@
         if (!silent && !callback.checkAndConfirmOutlyingDelete(parents, null))
             return null;
-        return new DeleteCommand(parents.iterator().next().getDataSet(), parents);
+        return new DeleteCommand(parents);
     }
 
@@ -457 +458 @@
                 .collect(Collectors.toSet());
         for (Relation cur : relationsToBeChanged) {
-            Relation rel = new Relation(cur);
-            rel.removeMembersFor(primitivesToDelete);
-            cmds.add(new ChangeCommand(cur, rel));
+            List<RelationMember> newMembers = cur.getMembers();
+            cur.getMembersFor(primitivesToDelete).forEach(newMembers::remove);
+            cmds.add(new ChangeMembersCommand(cur, newMembers));
         }
 
@@ -465 +466 @@
         //
         if (!primitivesToDelete.isEmpty()) {
-            cmds.add(new DeleteCommand(primitivesToDelete.iterator().next().getDataSet(), primitivesToDelete));
+            cmds.add(new DeleteCommand(primitivesToDelete));
         }