Subject: [PATCH] Fix #23399: Simplify way crashes

This was primarily caused by selection updates occurring while the commands were generated. The fixes could be any of the following:
* Synchronized collections
* New list
* Avoiding selection updates

The last option was taken for the following reasons:
* Avoiding a StackOverflow exception when many ways are being simplified
* Reduced memory allocations (>11GB to <250MB)
* Reduced CPU cycles
---
Index: core/src/org/openstreetmap/josm/actions/SimplifyWayAction.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/org/openstreetmap/josm/actions/SimplifyWayAction.java b/core/src/org/openstreetmap/josm/actions/SimplifyWayAction.java
--- a/core/src/org/openstreetmap/josm/actions/SimplifyWayAction.java	(revision 18934)
+++ b/core/src/org/openstreetmap/josm/actions/SimplifyWayAction.java	(date 1705002217139)
@@ -340,12 +340,21 @@
      * @since 16566 (private)
      */
     private static SequenceCommand buildSimplifyWaysCommand(List<Way> ways, double threshold) {
-        Collection<Command> allCommands = ways.stream()
+        List<Command> allCommands = ways.stream()
                 .map(way -> createSimplifyCommand(way, threshold))
                 .filter(Objects::nonNull)
                 .collect(StreamUtils.toUnmodifiableList());
         if (allCommands.isEmpty())
             return null;
+        final List<OsmPrimitive> deletedPrimitives = allCommands.stream()
+                .map(Command::getChildren)
+                .flatMap(Collection::stream)
+                .filter(DeleteCommand.class::isInstance)
+                .map(DeleteCommand.class::cast)
+                .map(DeleteCommand::getParticipatingPrimitives)
+                .flatMap(Collection::stream)
+                .collect(Collectors.toList());
+        allCommands.get(0).getAffectedDataSet().clearSelection(deletedPrimitives);
         return new SequenceCommand(
                 trn("Simplify {0} way", "Simplify {0} ways", allCommands.size(), allCommands.size()),
                 allCommands);
@@ -371,6 +380,18 @@
      * @since 15419
      */
     public static SequenceCommand createSimplifyCommand(Way w, double threshold) {
+        return createSimplifyCommand(w, threshold, true);
+    }
+
+    /**
+     * Creates the SequenceCommand to simplify a way with a given threshold.
+     *
+     * @param w the way to simplify
+     * @param threshold the max error threshold
+     * @param deselect {@code true} if we want to deselect the deleted nodes
+     * @return The sequence of commands to run
+     */
+    private static SequenceCommand createSimplifyCommand(Way w, double threshold, boolean deselect) {
         int lower = 0;
         int i = 0;
 
@@ -417,7 +438,9 @@
         Collection<Command> cmds = new LinkedList<>();
         cmds.add(new ChangeNodesCommand(w, newNodes));
         cmds.add(new DeleteCommand(w.getDataSet(), delNodes));
-        w.getDataSet().clearSelection(delNodes);
+        if (deselect) {
+            w.getDataSet().clearSelection(delNodes);
+        }
         return new SequenceCommand(
                 trn("Simplify Way (remove {0} node)", "Simplify Way (remove {0} nodes)", delNodes.size(), delNodes.size()), cmds);
     }
Index: core/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java b/core/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java
--- a/core/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java	(revision 18934)
+++ b/core/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java	(date 1705001941449)
@@ -1,9 +1,14 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.actions;
 
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
+import java.awt.GraphicsEnvironment;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -24,11 +29,16 @@
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.gui.ExtendedDialog;
 import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.IllegalDataException;
 import org.openstreetmap.josm.io.OsmReader;
 import org.openstreetmap.josm.testutils.annotations.Main;
 import org.openstreetmap.josm.testutils.annotations.Projection;
+import org.openstreetmap.josm.testutils.mockers.ExtendedDialogMocker;
+import org.openstreetmap.josm.testutils.mockers.HelpAwareOptionPaneMocker;
 import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -99,4 +109,34 @@
         assertEquals(1, deleteCommands.size());
         assertEquals(Collections.singleton(n1), deleteCommands.iterator().next().getParticipatingPrimitives());
     }
+
+    /**
+     * Non-regression test for #23399
+     */
+    @Test
+    void testNonRegression23399() {
+        TestUtils.assumeWorkingJMockit();
+        new ExtendedDialogMocker(Collections.singletonMap("Simplify way", "Simplify")) {
+            @Override
+            protected String getString(ExtendedDialog instance) {
+                return instance.getTitle();
+            }
+        };
+        new HelpAwareOptionPaneMocker(Collections.singletonMap("The selection contains 1 000 ways. Are you sure you want to simplify them all?", "Yes"));
+        final ArrayList<Way> ways = new ArrayList<>(1000);
+        final DataSet ds = new DataSet();
+        for (int i = 0; i < 1000; i++) {
+            final Way way = TestUtils.newWay("", new Node(new LatLon(0, 0)), new Node(new LatLon(0, 0.001)),
+                    new Node(new LatLon(0, 0.002)));
+            ways.add(way);
+            ds.addPrimitiveRecursive(way);
+        }
+        MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, "SimplifyWayActionTest#testNonRegression23399", null));
+        GuiHelper.runInEDTAndWait(() -> ds.setSelected(ds.allPrimitives()));
+        assertEquals(ds.allPrimitives().size(), ds.getAllSelected().size());
+        assertDoesNotThrow(() -> GuiHelper.runInEDTAndWaitWithException(() -> action.actionPerformed(null)));
+        assertAll(ways.stream().map(way -> () -> assertEquals(2, way.getNodesCount())));
+        assertAll(ds.getAllSelected().stream().map(p -> () -> assertFalse(p.isDeleted())));
+        assertEquals(3000, ds.getAllSelected().size());
+    }
 }