Changeset 19671 in josm

Timestamp:

2026-11-01T20:49:00+01:00 (8 hours ago)

Author:

gaben

Message:

fix #22878 - ConcurrentModificationException in MapCSSTagChecker when the rules are reloaded during validation

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

@@ -405 +405 @@
      */
     void visit(Collection<OsmPrimitive> selection, Predicate<String> urlPredicate) {
+        // the rules may be reloaded by another thread (preferences, file watcher) while the validation is running, see #22878
+        final List<Entry<String, Set<MapCSSTagCheckerRule>>> checksToRun;
+        synchronized (this) {
+            checksToRun = new ArrayList<>(checks.entrySet());
+        }
         if (urlPredicate == null && progressMonitor != null) {
-            progressMonitor.setTicksCount(selection.size() * checks.size());
+            progressMonitor.setTicksCount(selection.size() * checksToRun.size());
         }
 
@@ -414 +419 @@
 
         Set<OsmPrimitive> surrounding = new HashSet<>();
-        for (Entry<String, Set<MapCSSTagCheckerRule>> entry : checks.entrySet()) {
+        for (Entry<String, Set<MapCSSTagCheckerRule>> entry : checksToRun) {
             if (isCanceled()) {
                 break;

trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java

@@ -478 +478 @@
         assertTrue(test.getErrors().isEmpty());
     }
+
+    /**
+     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/22878">#22878</a>
+     * - rules reloaded while the validation is running.
+     * @throws ParseException if a parsing error occurs
+     */
+    @Test
+    void testTicket22878() throws ParseException {
+        final MapCSSTagChecker test = buildTagChecker("node[amenity=bench] { throwWarning: \"bench\"; }");
+        final String css = "node[amenity=bench] { throwWarning: \"other bench\"; }";
+        test.checks.putAll("other", MapCSSTagCheckerRule.readMapCSS(new StringReader(css)).parseChecks);
+        final OsmPrimitive p = OsmUtils.createPrimitive("node amenity=bench");
+        new DataSet(p);
+        test.startTest(NullProgressMonitor.INSTANCE);
+        // the predicate is called while iterating over the rules, reload them there like addMapCSS does
+        assertDoesNotThrow(() -> test.visit(List.of(p), url -> {
+            try {
+                test.checks.remove("other");
+                test.checks.putAll("other", MapCSSTagCheckerRule.readMapCSS(new StringReader(css)).parseChecks);
+            } catch (ParseException e) {
+                throw new IllegalStateException(e);
+            }
+            return true;
+        }));
+        test.endTest();
+        assertTrue(test.getErrors().stream().anyMatch(e -> "bench".equals(e.getMessage())));
+    }
 }