[PATCH] EDT violation when fixing UntaggedNode errors when no download bounds exist (overpass)

[gaben]

What steps will reproduce the problem?

See comment:1

Please provide any additional information below. Attach a screenshot if possible.

The log spans over 4000 lines, so I zipped and attached it instead.

Revision:18699
Is-Local-Build:true
Build-Date:2023-03-26 20:37:32

Identification: JOSM/1.5 (18699 SVN en) Linux Ubuntu 20.04.6 LTS
Memory Usage: 648 MB / 2480 MB (136 MB allocated, but free)
Java version: 11.0.18+10-post-Ubuntu-0ubuntu120.04.1, Ubuntu, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1914×1016 (scaling 1.00×1.00)
Maximum Screen Size: 1914×1016
Best cursor sizes: 16×16→16×16, 32×32→32×32
Environment variable LANG: hu_HU.UTF-8
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_HU
Numbers with default locale: 1234567890 -> 1234567890
Desktop environment: ubuntu:GNOME
Java package: openjdk-11-jre:amd64-11.0.18+10-0ubuntu1~20.04.1
Java ATK Wrapper package: libatk-wrapper-java:all-0.37.1-1
fonts-noto: fonts-noto:-
VM arguments: [-agentlib:jdwp=transport=dt_socket,address=127.0.0.1:55215,suspend=y,server=n, -javaagent:/home/username/progs/idea/plugins/java/lib/rt/debugger-agent.jar, -Dfile.encoding=UTF-8]
Dataset consistency test: No problems found

[taylor.smock]

It looks like it is the same thing 110 times.

EDT violation detected
org.openstreetmap.josm.gui.widgets.JMultilineLabel[,0,0,0x0,invalid,alignmentX=0.0,alignmentY=0.0,border=,flags=0,maximumSize=,minimumSize=,preferredSize=,caretColor=,disabledTextColor=,editable=true,margin=,selectedTextColor=,selectionColor=,kit=,typeHandlers=]
	at java.base/java.lang.Thread.getStackTrace(Thread.java:1602)
	at org.openstreetmap.josm.gui.util.CheckThreadViolationRepaintManager.checkThreadViolations(CheckThreadViolationRepaintManager.java:54)
	at org.openstreetmap.josm.gui.util.CheckThreadViolationRepaintManager.addDirtyRegion(CheckThreadViolationRepaintManager.java:45)
	at java.desktop/javax.swing.JComponent.repaint(JComponent.java:4836)
	at java.desktop/java.awt.Component.repaint(Component.java:3393)
	at java.desktop/javax.swing.text.JTextComponent.setEditable(JTextComponent.java:1818)
	at java.desktop/javax.swing.text.JTextComponent.<init>(JTextComponent.java:323)
	at java.desktop/javax.swing.JEditorPane.<init>(JEditorPane.java:198)
	at org.openstreetmap.josm.gui.widgets.JMultilineLabel.<init>(JMultilineLabel.java:62)
	at org.openstreetmap.josm.gui.widgets.JMultilineLabel.<init>(JMultilineLabel.java:48)
	at org.openstreetmap.josm.gui.widgets.JMultilineLabel.<init>(JMultilineLabel.java:35)
	at org.openstreetmap.josm.actions.JosmAction.showConfirmOutlyingOperationDialog(JosmAction.java:530)
	at org.openstreetmap.josm.actions.JosmAction.checkAndConfirmOutlyingOperation(JosmAction.java:516)
	at org.openstreetmap.josm.actions.DeleteAction.checkAndConfirmOutlyingDelete(DeleteAction.java:105)
	at org.openstreetmap.josm.actions.DeleteAction$1.checkAndConfirmOutlyingDelete(DeleteAction.java:44)
	at org.openstreetmap.josm.command.DeleteCommand.delete(DeleteCommand.java:436)
	at org.openstreetmap.josm.command.DeleteCommand.delete(DeleteCommand.java:356)
	at org.openstreetmap.josm.data.validation.Test.deletePrimitivesIfNeeded(Test.java:343)
	at org.openstreetmap.josm.data.validation.tests.UntaggedNode.fixError(UntaggedNode.java:107)
	at org.openstreetmap.josm.data.validation.TestError.getFix(TestError.java:524)
	at org.openstreetmap.josm.gui.dialogs.ValidatorDialog$FixTask.fixError(ValidatorDialog.java:630)
	at org.openstreetmap.josm.gui.dialogs.ValidatorDialog$FixTask.realRun(ValidatorDialog.java:658)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.doRealRun(PleaseWaitRunnable.java:94)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.run(PleaseWaitRunnable.java:142)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

$ cat ~/Downloads/EDT\ violations.log| grep 'CheckThreadViolationRepaintManager.checkThreadViolations' | wc -l
  110
$ cat ~/Downloads/EDT\ violations.log| grep 'org.openstreetmap.josm.data.validation.TestError.getFix' | wc -l
  110
$ cat ~/Downloads/EDT\ violations.log| grep 'org.openstreetmap.josm.data.validation.tests.UntaggedNode.fixError' | wc -l
  110

Steps to reproduce:

1. Download only nodes via overpass, e.g.

   [out:xml][timeout:90][bbox:{{bbox}}];
   (
     node;
   );
   (._;>;);
   out meta;
   

2. Run the JOSM validator (note: I recommend doing this on a single node instead of the entire dataset)
3. Use the Fix command

[taylor.smock]

src/org/openstreetmap/josm/actions/DeleteAction.java

@@ -23 +23 @@
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.MapFrame;
 import org.openstreetmap.josm.gui.dialogs.DeleteFromRelationConfirmationDialog;
+import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.gui.widgets.JMultilineLabel;
 import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Shortcut;
@@ -102 +103 @@
      */
     public static boolean checkAndConfirmOutlyingDelete(Collection<? extends OsmPrimitive> primitives,
             Collection<? extends OsmPrimitive> ignore) {
-        return checkAndConfirmOutlyingOperation("delete",
+        return Boolean.TRUE.equals(GuiHelper.runInEDTAndWaitAndReturn(() -> checkAndConfirmOutlyingOperation("delete",
                 tr("Delete confirmation"),
                 tr("You are about to delete nodes which can have other referrers not yet downloaded."
                         + "<br>"
@@ -113 +114 @@
                         + "<br>"
                         + "This will cause problems because you don''t see the real object."
                         + "<br>" + "Do you really want to delete?"),
-                primitives, ignore);
+                primitives, ignore)));
     }
 
     /**

[gaben]

Replying to taylor.smock:

It looks like it is the same thing 110 times.

Uploaded the full log, to help the discovery. I have close to zero knowledge in Java GUI programming as I've only used JFX, once, for a university project :)

Thanks for the investigation!

[taylor.smock]

Replying to gaben:

Uploaded the full log, to help the discovery. I have close to zero knowledge in Java GUI programming as I've only used JFX, once, for a university project :)

No worries. If it helps, many UI platforms want all UI drawing to happen on the same thread (for swing, that is often called the "Event Dispatch Thread" or "EDT"). JOSM has a custom listener that logs to console the stacktrace when something triggers a paint call outside of the EDT.

In this case, it occurs because we try to run the fix code in a non-EDT thread, but (at least when all data is downloaded from overpass), there are no download bounds, which means that JOSM wants to ask the user if they should delete the node.

I'm not certain we should be running that check on nodes that are on layers without a download area. So something else (besides what I posted) might be necessary.

[skyper]

See #22847.

[taylor.smock]

Ticket retargeted after milestone closed

[taylor.smock]

Ticket retargeted after milestone closed

[taylor.smock]

In 18756/josm:

Fix #22845: EDT violation when fixing UntaggedNode errors when no download bounds exist