Changeset 17359 in josm

Timestamp:

2020-11-25T13:23:30+01:00 (4 years ago)

Author:

GerdP

Message:

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

• revert changes made in r17358 in SplitWayCommand , they break a unit test and I do not yet see why

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;
@@ -91 +90 @@
      * @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 #getNewWays})
+     * @param newWays The resulting new ways (which is saved for later retrieval with {@link #getOriginalWay})
      */
     public SplitWayCommand(String name, Collection<Command> commandList,
@@ -403 +402 @@
                 } 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);
-                break;
+                return Optional.of(splitBasedOnAnalyses(way, newWays, newSelection, analysis, indexOfWayToKeep));
             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.
-                break;
+                return Optional.of(splitBasedOnAnalyses(way, newWays, newSelection, analysis, indexOfWayToKeep));
             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();
         }
     }
@@ -475 +465 @@
                     }
                     if (c == null) {
-                        c = new Relation(r); // #19885: will be removed later
+                        c = new Relation(r);
                     }
 
@@ -562 +552 @@
                 }
             }
+
+            if (c != null) {
+                commandList.add(new ChangeCommand(r.getDataSet(), r, c));
+            }
         }
         changedWay.setNodes(null); // see #19885
@@ -581 +575 @@
             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);
-            }
         }
 
@@ -704 +688 @@
         }
 
-        Set<Relation> modifiedRelations = new LinkedHashSet<>();
         // Perform the split.
         for (RelationAnalysis relationAnalysis : analysis.getRelationAnalyses()) {
@@ -746 +729 @@
                 }
             }
-            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
         }