Changeset 17392 in josm for trunk/src

Timestamp:

2020-12-07T10:58:03+01:00 (3 years ago)

Author:

GerdP

Message:

fix #19098: Unable to update plugin after crash of plugin

• wait for completion of threads
• don't start PluginHandler.updateOrdisablePluginAfterException(e) in EDT

tested with old buildings_tool.jar version 34113 which causes an immediate crash when a new data layer is created.
TODO: Why didn't any unit test find this obvious bug? Seems we are mocking too much?

Location:

trunk/src/org/openstreetmap/josm

Files:

• gui/bugreport/BugReportDialog.java (modified) (1 diff)
• plugins/PluginHandler.java (modified) (5 diffs)

trunk/src/org/openstreetmap/josm/gui/bugreport/BugReportDialog.java

@@ -230 +230 @@
             return SuppressionMode.NONE;
         } else {
+            PluginDownloadTask downloadTask = PluginHandler.updateOrdisablePluginAfterException(e);
             return GuiHelper.runInEDTAndWaitAndReturn(() -> {
-                PluginDownloadTask downloadTask = PluginHandler.updateOrdisablePluginAfterException(e);
                 if (downloadTask != null) {
                     // Ask for restart to install new plugin

trunk/src/org/openstreetmap/josm/plugins/PluginHandler.java

@@ -38 +38 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.TimeUnit;
@@ -954 +955 @@
         }
         try {
+            Map<String, PluginInformation> ret = new HashMap<>();
             ReadLocalPluginInformationTask task = new ReadLocalPluginInformationTask(monitor);
+            Future<?> future = MainApplication.worker.submit(task);
             try {
-                task.run();
-            } catch (RuntimeException e) { // NOPMD
+                future.get();
+            } catch (ExecutionException e) {
                 Logging.error(e);
-                return null;
-            }
-            Map<String, PluginInformation> ret = new HashMap<>();
+                return ret;
+            } catch (InterruptedException e) {
+                Logging.warn("InterruptedException in " + PluginHandler.class.getSimpleName()
+                        + " while loading locally available plugin information");
+                return ret;
+            }
             for (PluginInformation pi: task.getAvailablePlugins()) {
                 ret.put(pi.name, pi);
@@ -1118 +1124 @@
                     monitor.createSubTaskMonitor(1, false),
                     Preferences.main().getOnlinePluginSites(), displayErrMsg
-            );
-            task1.run();
-            List<PluginInformation> allPlugins = task1.getAvailablePlugins();
+                    );
+            List<PluginInformation> allPlugins = null;
+            Future<?> future = MainApplication.worker.submit(task1);
 
             try {
+                future.get();
+                allPlugins = task1.getAvailablePlugins();
                 plugins = buildListOfPluginsToLoad(parent, monitor.createSubTaskMonitor(1, false));
                 // If only some plugins have to be updated, filter the list
@@ -1129 +1137 @@
                     plugins = SubclassFilteredCollection.filter(plugins, pi -> pluginsWantedName.contains(pi.name));
                 }
-            } catch (RuntimeException e) { // NOPMD
-                Logging.warn(tr("Failed to download plugin information list"));
+            } catch (ExecutionException e) {
+                Logging.warn(tr("Failed to download plugin information list") + ": ExecutionException");
                 Logging.error(e);
+                // don't abort in case of error, continue with downloading plugins below
+            } catch (InterruptedException e) {
+                Logging.warn(tr("Failed to download plugin information list") + ": InterruptedException");
                 // don't abort in case of error, continue with downloading plugins below
             }
@@ -1170 +1181 @@
                         pluginsToDownload,
                         tr("Update plugins")
-                );
+                        );
+                future = MainApplication.worker.submit(pluginDownloadTask);
+
                 try {
-                    pluginDownloadTask.run();
-                } catch (RuntimeException e) { // NOPMD
+                    future.get();
+                } catch (ExecutionException e) {
                     Logging.error(e);
+                    alertFailedPluginUpdate(parent, pluginsToUpdate);
+                    return plugins;
+                } catch (InterruptedException e) {
+                    Logging.warn("InterruptedException in " + PluginHandler.class.getSimpleName()
+                            + " while updating plugins");
                     alertFailedPluginUpdate(parent, pluginsToUpdate);
                     return plugins;