Changeset 17358 in josm for trunk/src/org/openstreetmap/josm/command/SplitWayCommand.java

Timestamp:

2020-11-25T11:50:22+01:00 (3 years ago)

Author:

GerdP

Message:

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

• (hopefully) fix memory leaks in complex actions
• handle complex cases with presets and RelationEditor

I hope these changes don't break plugins which extend or overwrite RelationEditor

File:

• trunk/src/org/openstreetmap/josm/command/SplitWayCommand.java (modified) (8 diffs)

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

@@ -17 +17 @@
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -90 +91 @@
      * @param newSelection The new list of selected primitives ids (which is saved for later retrieval with {@link #getNewSelection})
      * @param originalWay The original way being split (which is saved for later retrieval with {@link #getOriginalWay})
-     * @param newWays The resulting new ways (which is saved for later retrieval with {@link #getOriginalWay})
+     * @param newWays The resulting new ways (which is saved for later retrieval with {@link #getNewWays})
      */
     public SplitWayCommand(String name, Collection<Command> commandList,
@@ -402 +403 @@
                 } catch (OsmTransferException e) {
                     ExceptionDialogUtil.explainException(e);
+                    analysis.cleanup();
                     return Optional.empty();
                 }
                 // If missing relation members were downloaded, perform the analysis again to find the relation
                 // member order for all relations.
+                analysis.cleanup();
                 analysis = analyseSplit(way, wayToKeep, newWays);
-                return Optional.of(splitBasedOnAnalyses(way, newWays, newSelection, analysis, indexOfWayToKeep));
+                break;
             case GO_AHEAD_WITHOUT_DOWNLOADS:
                 // Proceed with the split with the information we have.
                 // This can mean that there are no missing members we want, or that the user chooses to continue
                 // the split without downloading them.
-                return Optional.of(splitBasedOnAnalyses(way, newWays, newSelection, analysis, indexOfWayToKeep));
+                break;
             case USER_ABORTED:
             default:
                 return Optional.empty();
+        }
+        try {
+            return Optional.of(splitBasedOnAnalyses(way, newWays, newSelection, analysis, indexOfWayToKeep));
+        } finally {
+            // see #19885
+            wayToKeep.setNodes(null);
+            analysis.cleanup();
         }
     }
@@ -465 +475 @@
                     }
                     if (c == null) {
-                        c = new Relation(r);
+                        c = new Relation(r); // #19885: will be removed later
                     }
 
@@ -552 +562 @@
                 }
             }
-
-            if (c != null) {
-                commandList.add(new ChangeCommand(r.getDataSet(), r, c));
-            }
         }
         changedWay.setNodes(null); // see #19885
@@ -575 +581 @@
             warningTypes = warnings;
             this.numberOfRelations = numberOfRelations;
+        }
+
+        /**
+         * Unlink temporary copies of relations. See #19885
+         */
+        void cleanup() {
+            for (RelationAnalysis ra : relationAnalyses) {
+                if (ra.relation.getDataSet() == null)
+                    ra.relation.setMembers(null);
+            }
         }
 
@@ -688 +704 @@
         }
 
+        Set<Relation> modifiedRelations = new LinkedHashSet<>();
         // Perform the split.
         for (RelationAnalysis relationAnalysis : analysis.getRelationAnalyses()) {
@@ -729 +746 @@
                 }
             }
+            modifiedRelations.add(relation);
+        }
+        for (Relation r : modifiedRelations) {
+            DataSet ds = way.getDataSet();
+            Relation orig = (Relation) ds.getPrimitiveById(r);
+            analysis.getCommands().add(new ChangeMembersCommand(orig, new ArrayList<>(r.getMembers())));
+            r.setMembers(null); // see #19885
         }