Fix #22233: Deadlock when uploading with incomplete member download e…

…nabled

This is caused by custom threading and blocking during the
validation process on upload.

The validation is run in the EDT, but the custom threading and blocking
created a new thread, started it, and then just blocked the EDT. The
new thread would then attempt to download data, and during that process
it would attempt to show a UI window. This is where the deadlock would
occur (the EDT thread is blocked on this new thread).

Signed-off-by: Taylor Smock <[email protected]>

.tx/config

@@ -1,9 +1,9 @@
 [main]
 host = https://www.transifex.com
 
-[josm.josm-plugin_pt_assistant]
+[o:josm:p:josm:r:josm-plugin_pt_assistant]
 file_filter = src/main/po/<lang>.po
-lang_map = ca@valencia: ca-valencia
+lang_map = ca@valencia: ca-valencia, sr@latin: sr-latin
 minimum_perc=40
 source_file = build/i18n/pot/josm-plugin_pt_assistant.pot
 source_lang = en

src/main/java/org/openstreetmap/josm/plugins/pt_assistant/PTAssistantPluginPreferences.java

@@ -47,6 +47,7 @@ public PTAssistantPluginPreferences() {
 
         // CHECKSTYLE.OFF: LineLength
         downloadIncompleteMembers = new JCheckBox(tr("Download incomplete route relation members"));
+        downloadIncompleteMembers.setToolTipText(tr("Incomplete route relations will be downloaded on validation start, but may not be available for validation until the next validation run."));
         stopArea = new JCheckBox(tr("Include stop_area tests"));
         transferDetails = new JCheckBox(tr("Remove public_transport=platform from platform ways"));
         optionsForMendAction = new JCheckBox(tr("Use 1/2/3/4 instead of A/B/C/D for selecting which way to follow"));

src/main/java/org/openstreetmap/josm/plugins/pt_assistant/actions/IncompleteMembersDownloadRunnable.java

@@ -0,0 +1,84 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.plugins.pt_assistant.actions;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import javax.swing.SwingUtilities;
+
+import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
+import org.openstreetmap.josm.data.osm.PrimitiveId;
+import org.openstreetmap.josm.data.osm.Relation;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.io.DownloadPrimitivesWithReferrersTask;
+import org.openstreetmap.josm.plugins.pt_assistant.utils.RouteUtils;
+import org.openstreetmap.josm.plugins.pt_assistant.utils.StopUtils;
+
+/**
+ * Runnable that downloads incomplete relation members while pausing the rest of testing
+ * @author darya
+ *
+ */
+public class IncompleteMembersDownloadRunnable implements Runnable {
+
+    /**
+     * Default constructor
+     */
+    public IncompleteMembersDownloadRunnable() {
+        super();
+    }
+
+    @Override
+    public void run() {
+
+        synchronized (this) {
+
+            ArrayList<PrimitiveId> list = new ArrayList<>();
+
+            // if there are selected routes, try adding them first:
+            for (Relation currentSelectedRelation : MainApplication.getLayerManager().getEditDataSet().getSelectedRelations()) {
+                if (RouteUtils.isVersionTwoPTRoute(currentSelectedRelation)) {
+                    list.add(currentSelectedRelation);
+                }
+            }
+
+            if (list.isEmpty()) {
+                // add all route relations that are of public_transport
+                // version 2:
+                Collection<Relation> allRelations = MainApplication.getLayerManager().getEditDataSet().getRelations();
+                for (Relation currentRelation : allRelations) {
+                    if (RouteUtils.isVersionTwoPTRoute(currentRelation)) {
+                        list.add(currentRelation);
+                    }
+                }
+            }
+
+            // add all stop_positions:
+            Collection<Node> allNodes = MainApplication.getLayerManager().getEditDataSet().getNodes();
+            for (Node currentNode : allNodes) {
+                if (StopUtils.isStopPosition(currentNode)) {
+                    List<OsmPrimitive> referrers = currentNode.getReferrers();
+                    boolean parentWayExists = false;
+                    for (OsmPrimitive referrer : referrers) {
+                        if (referrer.getType() == OsmPrimitiveType.WAY) {
+                            parentWayExists = true;
+                            break;
+                        }
+                    }
+                    if (!parentWayExists) {
+                        list.add(currentNode);
+
+                    }
+
+                }
+            }
+
+            DownloadPrimitivesWithReferrersTask task = new DownloadPrimitivesWithReferrersTask(false, list, false, true,
+                null, null);
+            task.run();
+        }
+    }
+}

src/main/java/org/openstreetmap/josm/plugins/pt_assistant/validation/PTAssistantValidatorTest.java

@@ -8,6 +8,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Map.Entry;
+import java.util.concurrent.ExecutionException;
 
 import javax.swing.JOptionPane;
 import javax.swing.SwingUtilities;
@@ -25,11 +26,12 @@
 import org.openstreetmap.josm.data.validation.Severity;
 import org.openstreetmap.josm.data.validation.Test;
 import org.openstreetmap.josm.data.validation.TestError;
+import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.plugins.pt_assistant.PTAssistantPlugin;
 import org.openstreetmap.josm.plugins.pt_assistant.actions.FixTask;
-import org.openstreetmap.josm.plugins.pt_assistant.actions.IncompleteMembersDownloadThread;
+import org.openstreetmap.josm.plugins.pt_assistant.actions.IncompleteMembersDownloadRunnable;
 import org.openstreetmap.josm.plugins.pt_assistant.data.PTRouteDataManager;
 import org.openstreetmap.josm.plugins.pt_assistant.data.PTRouteSegment;
 import org.openstreetmap.josm.plugins.pt_assistant.data.PTStop;
@@ -189,33 +191,23 @@ private boolean downloadIncompleteMembers() {
 
             } else {
 
-                SwingUtilities.invokeAndWait(() -> {
-                    try {
-                        userSelection[0] = showIncompleteMembersDownloadDialog();
-                    } catch (InterruptedException e) {
-                        Logging.error(e);
-                    }
-
-                });
+                SwingUtilities.invokeAndWait(() -> userSelection[0] = showIncompleteMembersDownloadDialog());
 
             }
 
-        } catch (InterruptedException | InvocationTargetException e) {
+        } catch (InterruptedException e) {
+            Logging.error(e);
+            Thread.currentThread().interrupt();
+            return false;
+        } catch(InvocationTargetException e) {
+            Logging.error(e);
             return false;
         }
 
         if (userSelection[0] == JOptionPane.YES_OPTION) {
 
-            Thread t = new IncompleteMembersDownloadThread();
-            t.start();
-            synchronized (t) {
-                try {
-                    t.wait();
-                } catch (InterruptedException e) {
-                    return false;
-                }
-            }
-
+            IncompleteMembersDownloadRunnable runnable = new IncompleteMembersDownloadRunnable();
+            runnable.run();
         }
 
         return true;
@@ -226,10 +218,8 @@ private boolean downloadIncompleteMembers() {
      * Shows the dialog asking the user about an incomplete member download
      *
      * @return user's selection
-     * @throws InterruptedException
-     *             if interrupted
      */
-    private int showIncompleteMembersDownloadDialog() throws InterruptedException {
+    private static int showIncompleteMembersDownloadDialog() {
         return PTProperties.DOWNLOAD_INCOMPLETE.get() ? JOptionPane.YES_OPTION : JOptionPane.NO_OPTION;
     }