Changeset 10999 in josm

Timestamp:

2016-09-14T12:44:16+02:00 (8 years ago)

Author:

michael2402

Message:

Force resetState() to run in the EDT thread to prevent deadlocks. Fixes #13632

Location:

trunk/src/org/openstreetmap/josm/gui/layer

Files:

• LayerManager.java (modified) (5 diffs)
• MainLayerManager.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/gui/layer/LayerManager.java

@@ -21 +21 @@
  * This manager handles a list of layers with the first layer being the front layer.
  * <h1>Threading</h1>
+ * Synchronization of the layer manager is done by synchronizing all read/write access. All changes are internally done in the EDT thread.
+ *
  * Methods of this manager may be called from any thread in any order.
- * Listeners are called while this layer manager is locked, so they should not block.
+ * Listeners are called while this layer manager is locked, so they should not block on other threads.
  *
  * @author Michael Zangl
@@ -35 +37 @@
          * Notifies this listener that a layer has been added.
          * <p>
-         * Listeners are called in the EDT thread.
+         * Listeners are called in the EDT thread. You should not do blocking or long-running tasks in this method.
          * @param e The new added layer event
          */
@@ -45 +47 @@
          * Listeners are called in the EDT thread after the layer was removed.
          * Use {@link LayerRemoveEvent#scheduleRemoval(Collection)} to remove more layers.
+         * You should not do blocking or long-running tasks in this method.
          * @param e The layer to be removed (as event)
          */
@@ -52 +55 @@
          * Notifies this listener that the order of layers was changed.
          * <p>
-         * Listeners are called in the EDT thread and you can manipulate the layer manager in the current thread.
+         * Listeners are called in the EDT thread.
+         *  You should not do blocking or long-running tasks in this method.
          * @param e The order change event.
          */
@@ -418 +422 @@
      * @since 10432
      */
-    public synchronized void resetState() {
-        // some layer remove listeners remove other layers.
+    public void resetState() {
+        // we force this on to the EDT Thread to have a clean synchronization
+        // The synchronization lock needs to be held by the EDT.
+        GuiHelper.runInEDTAndWaitWithException(this::realResetState);
+    }
+
+    protected synchronized void realResetState() {
+        // The listeners trigger the removal of other layers
         while (!getLayers().isEmpty()) {
             removeLayer(getLayers().get(0));

trunk/src/org/openstreetmap/josm/gui/layer/MainLayerManager.java

@@ -372 +372 @@
 
     @Override
-    public synchronized void resetState() {
+    protected synchronized void realResetState() {
         // active and edit layer are unset automatically
-        super.resetState();
+        super.realResetState();
 
         activeLayerChangeListeners.clear();