Changeset 36217 in osm for applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/TheRing.java

Timestamp:

2024-03-03T10:25:25+01:00 (22 months ago)

Author:

GerdP

Message:

fix #23521: fix some memory leaks

• dispose dialogs
• either avoid to create clones of ways or relations or use setNodes(null) / setMembers(null)
• replaces most ChangeCommand instances by better specialized alternatives
• add some comments
• fix some checkstyle / sonar issues

File:

• applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/TheRing.java (modified) (8 diffs)

applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/TheRing.java

@@ -15 +15 @@
 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.ChangePropertyCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.DeleteCommand;
@@ -330 +333 @@
     public List<Command> getCommands(boolean createMultipolygon, Map<Relation, Relation> relationChangeMap) {
         Way sourceCopy = new Way(source);
+        Map<String, String> tagsToRemove = new HashMap<>();
         if (createMultipolygon) {
             Collection<String> linearTags = Config.getPref().getList(PREF_MULTIPOLY + "lineartags",
@@ -335 +339 @@
             relation = new Relation();
             relation.put("type", "multipolygon");
-            for (String key : sourceCopy.keySet()) {
-                if (linearTags.contains(key)) {
+            for (String key : source.keySet()) {
+                if (linearTags.contains(key)
+                        || ("natural".equals(key) && "coastline".equals(source.get("natural"))))
                     continue;
-                }
-                if ("natural".equals(key) && "coastline".equals(sourceCopy.get("natural"))) {
-                    continue;
-                }
-                relation.put(key, sourceCopy.get(key));
+                
+                relation.put(key, source.get(key));
                 sourceCopy.remove(key);
+                tagsToRemove.put(key, null);
             }
         }
@@ -354 +357 @@
                 Relation rel;
                 if (relationChangeMap != null) {
-                    if (relationChangeMap.containsKey(p)) {
-                        rel = relationChangeMap.get(p);
-                    } else {
+                    rel = relationChangeMap.get(p);
+                    if (rel == null) {
                         rel = new Relation((Relation) p);
                         relationChangeMap.put((Relation) p, rel);
@@ -362 +364 @@
                 } else {
                     rel = new Relation((Relation) p);
-                    relationCommands.add(new ChangeCommand(p, rel));
+                    relationCommands.add(new ChangeCommand(p, rel)); // should not happen 
                 }
                 for (int i = 0; i < rel.getMembersCount(); i++) {
@@ -384 +386 @@
                 if (createMultipolygon || !seg.getWayNodes().equals(source.getNodes())) {
                     sourceCopy.setNodes(seg.getWayNodes());
-                    commands.add(new ChangeCommand(source, sourceCopy));
+                    if (!tagsToRemove.isEmpty()) {
+                        commands.add(new ChangePropertyCommand(Collections.singleton(source), tagsToRemove));
+                    }
+                    if (!sourceCopy.getNodes().equals(source.getNodes()))
+                        commands.add(new ChangeNodesCommand(source, sourceCopy.getNodes()));
                 }
                 foundOwnWay = true;
@@ -398 +404 @@
             }
         }
+        sourceCopy.setNodes(null); // see #19885
         if (!foundOwnWay) {
             final Command deleteCommand = DeleteCommand.delete(Collections.singleton(source));
@@ -413 +420 @@
     public static void updateCommandsWithRelations(List<Command> commands, Map<Relation, Relation> relationCache) {
         for (Map.Entry<Relation, Relation> entry : relationCache.entrySet()) {
-            commands.add(new ChangeCommand(entry.getKey(), entry.getValue()));
+            Relation oldRel = entry.getKey();
+            Relation newRel = entry.getValue();
+            if (oldRel.getKeys().equals(newRel.getKeys())) {
+                commands.add(new ChangeMembersCommand(oldRel, newRel.getMembers()));
+                newRel.setMembers(null); // see #19885
+            } else {
+                commands.add(new ChangeCommand(oldRel, newRel)); // should not happen
+            }
         }
     }