Changeset 31404 in osm for applications/editors/josm/plugins

Timestamp:

2015-07-24T21:55:14+02:00 (9 years ago)

Author:

nokutu

Message:

Fixed #11716. Concurrency bug related with semiautomatic mode when closing the layer

Location:

applications/editors/josm/plugins/mapillary

Files:

• build.gradle (modified) (1 diff)
• src/org/openstreetmap/josm/plugins/mapillary/MapillaryData.java (modified) (2 diffs)
• src/org/openstreetmap/josm/plugins/mapillary/MapillaryLayer.java (modified) (8 diffs)
• src/org/openstreetmap/josm/plugins/mapillary/downloads/MapillaryDownloader.java (modified) (4 diffs)
• src/org/openstreetmap/josm/plugins/mapillary/downloads/MapillarySquareDownloadManagerThread.java (modified) (1 diff)
• src/org/openstreetmap/josm/plugins/mapillary/mode/AbstractMode.java (modified) (2 diffs)
• src/org/openstreetmap/josm/plugins/mapillary/mode/SelectMode.java (modified) (2 diffs)

applications/editors/josm/plugins/mapillary/build.gradle

@@ -138 +138 @@
 
 task runJosm(type: Exec) {
- commandLine 'josm'
+ commandLine 'java', '-jar', '/home/nokutu/josm/core/dist/josm-custom.jar'
 }
 runJosm.dependsOn installPluginToJosm

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryData.java

@@ -29 +29 @@
   private final List<MapillaryAbstractImage> multiSelectedImages;
 
-  private List<MapillaryDataListener> listeners = new CopyOnWriteArrayList<>();
+  private CopyOnWriteArrayList<MapillaryDataListener> listeners = new CopyOnWriteArrayList<>();
 
   /**
@@ -52 +52 @@
     }
     return INSTANCE;
+  }
+
+  /**
+   * Destroys the unique instance.
+   */
+  public void destroy() {
+    INSTANCE = null;
   }
 

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryLayer.java

@@ -54 +54 @@
 import java.util.List;
 import java.util.ArrayList;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
@@ -83 +84 @@
 
   /** The bounds of the areas for which the pictures have been downloaded */
-  public ArrayList<Bounds> bounds;
+  public CopyOnWriteArrayList<Bounds> bounds;
 
   /** Mode of the layer */
@@ -98 +99 @@
     super(tr("Mapillary Images"));
     data = MapillaryData.getInstance();
-    bounds = new ArrayList<>();
+    bounds = new CopyOnWriteArrayList<>();
     init();
   }
@@ -120 +121 @@
         MapillaryMainDialog.getInstance().getButton().doClick();
     }
-
     createHatchTexture();
     data.dataUpdated();
@@ -138 +138 @@
     }
     this.mode = mode;
-    Main.map.mapView.setNewCursor(mode.cursor, this);
-    Main.map.mapView.addMouseListener(mode);
-    Main.map.mapView.addMouseMotionListener(mode);
-    NavigatableComponent.addZoomChangeListener(mode);
-    updateHelpText();
+    if (mode != null) {
+      Main.map.mapView.setNewCursor(mode.cursor, this);
+      Main.map.mapView.addMouseListener(mode);
+      Main.map.mapView.addMouseMotionListener(mode);
+      NavigatableComponent.addZoomChangeListener(mode);
+      updateHelpText();
+    }
   }
 
@@ -168 +170 @@
   @Override
   public void destroy() {
+    setMode(null);
+    MapillaryDownloader.stopAll();
     MapillaryMainDialog.getInstance().setImage(null);
     MapillaryMainDialog.getInstance().updateImage();
-    data.getImages().clear();
-    MapillaryLayer.INSTANCE = null;
-    MapillaryData.INSTANCE = null;
     MapillaryPlugin.setMenuEnabled(MapillaryPlugin.EXPORT_MENU, false);
     MapillaryPlugin.setMenuEnabled(MapillaryPlugin.ZOOM_MENU, false);
@@ -180 +181 @@
     if (Main.map.mapView.getEditLayer() != null)
       Main.map.mapView.getEditLayer().data.removeDataSetListener(this);
+    MapillaryLayer.INSTANCE = null;
     super.destroy();
   }
@@ -593 +595 @@
     else
       ret += tr("No images found");
-    ret += " -- " + tr(mode.toString());
-
+    if (mode != null)
+      ret += " -- " + tr(mode.toString());
     Main.map.statusLine.setHelpText(ret);
   }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/downloads/MapillaryDownloader.java

@@ -3 +3 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 
 import javax.swing.JOptionPane;
@@ -34 +35 @@
   public final static String CLIENT_ID = "NzNRM2otQkR2SHJzaXJmNmdQWVQ0dzo1YTA2NmNlODhlNWMwOTBm";
   /** Executor that will run the petitions */
-  public final static Executor EXECUTOR = Executors.newSingleThreadExecutor();
+  public final static ThreadPoolExecutor EXECUTOR = new ThreadPoolExecutor(1,
+      1, 100, TimeUnit.SECONDS, new ArrayBlockingQueue<Runnable>(50));
 
   /**
@@ -51 +53 @@
     queryStringParts.put("max_lat", maxLatLon.lat());
     queryStringParts.put("max_lon", maxLatLon.lon());
+    run(new MapillarySquareDownloadManagerThread(queryStringParts,
+        MapillaryLayer.getInstance()));
+  }
 
-    try {
-      EXECUTOR.execute(new MapillarySquareDownloadManagerThread(
-          queryStringParts, MapillaryLayer.getInstance()));
-    } catch (Exception e) {
-      Main.error(e);
-    }
+  private static void run(Thread t) {
+    EXECUTOR.execute(t);
   }
 
@@ -130 +131 @@
     }
   }
+
+  /**
+   * Stops all running threads.
+   */
+  public static void stopAll() {
+    EXECUTOR.shutdownNow();
+  }
 }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/downloads/MapillarySquareDownloadManagerThread.java

@@ -83 +83 @@
       }
     } catch (InterruptedException e) {
-      Main.error(e);
+      Main.error("Mapillary download interrupted (probably because of closing the layer).");
     }
     layer.updateHelpText();

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/mode/AbstractMode.java

@@ -27 +27 @@
 
   protected MapillaryData data = MapillaryData.getInstance();
-
-  /** If in semiautomatic mode, the last Epoch time when there was a download */
-  private long lastDownload;
+  private static final SemiautomaticThread semiautomaticThread = new SemiautomaticThread();
 
   /**
@@ -63 +61 @@
 
   @Override
-  public synchronized void zoomChanged() {
+  public void zoomChanged() {
     if (Main.pref.get("mapillary.download-mode").equals(
         MapillaryDownloader.MODES[1])
         || MapillaryLayer.getInstance().TEMP_SEMIAUTOMATIC) {
-      if (Calendar.getInstance().getTimeInMillis() - lastDownload >= 2000) {
-        lastDownload = Calendar.getInstance().getTimeInMillis();
-        MapillaryDownloader.completeView();
-      } else {
-        new Thread() {
-          @Override
-          public synchronized void run() {
-            try {
-              wait(100);
-            } catch (InterruptedException e) {
-            }
-            zoomChanged();
+      if (!semiautomaticThread.isAlive())
+        semiautomaticThread.start();
+      semiautomaticThread.moved();
+    }
+  }
+
+  private static class SemiautomaticThread extends Thread {
+
+    /** If in semiautomatic mode, the last Epoch time when there was a download */
+    private long lastDownload;
+
+    private boolean moved = false;
+
+    @Override
+    public void run() {
+      while (true) {
+        if (moved
+            && Calendar.getInstance().getTimeInMillis() - lastDownload >= 2000) {
+          lastDownload = Calendar.getInstance().getTimeInMillis();
+          MapillaryDownloader.completeView();
+          moved = false;
+          MapillaryData.getInstance().dataUpdated();
+        }
+        synchronized (this) {
+          try {
+            wait(100);
+          } catch (InterruptedException e) {
+            Main.error(e);
           }
-        }.start();
+        }
       }
+    }
+
+    public void moved() {
+      moved = true;
     }
   }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/mode/SelectMode.java

@@ -18 +18 @@
 import org.openstreetmap.josm.plugins.mapillary.commands.CommandTurnImage;
 import org.openstreetmap.josm.plugins.mapillary.commands.MapillaryRecord;
+import org.openstreetmap.josm.plugins.mapillary.downloads.MapillaryDownloader;
 import org.openstreetmap.josm.plugins.mapillary.gui.MapillaryMainDialog;
 
@@ -41 +42 @@
   public SelectMode() {
     record = MapillaryRecord.getInstance();
+    if (Main.pref.get("mapillary.download-mode").equals(
+        MapillaryDownloader.MODES[1]))
+      zoomChanged();
   }