Changeset 18549 in josm for trunk/src/org/openstreetmap

Timestamp:

2022-09-06T18:40:21+02:00 (2 years ago)

Author:

taylor.smock

Message:

See #22183: NoClassDefFoundError: Could not initialize class org.openstreetmap.josm.actions.SessionSaveAction

The root cause is a suppressed NPE whose stack trace is not given
in the bug report.

This adds a method to BugReport to add suppressed exceptions to a report.
In the future, if we keep the code long-term, methods in Logging which take
exceptions may assume the exception is suppressed, and add it to the
suppressed exceptions.

Location:

trunk/src/org/openstreetmap/josm

Files:

• gui/progress/swing/ProgressMonitorExecutor.java (modified) (2 diffs)
• gui/util/GuiHelper.java (modified) (1 diff)
• tools/bugreport/BugReport.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/gui/progress/swing/ProgressMonitorExecutor.java

@@ -11 +11 @@
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Utils;
+import org.openstreetmap.josm.tools.bugreport.BugReport;
 
 /**
@@ -59 +60 @@
         if (t != null) {
             Logging.error("Thread {0} raised {1}", Thread.currentThread().getName(), t);
+            BugReport.addSuppressedException(t);
         }
     }

trunk/src/org/openstreetmap/josm/gui/util/GuiHelper.java

@@ -212 +212 @@
     static void handleEDTException(Throwable t) {
         Logging.logWithStackTrace(Logging.LEVEL_ERROR, t, "Exception raised in EDT");
+        BugReport.addSuppressedException(t);
     }
 

trunk/src/org/openstreetmap/josm/tools/bugreport/BugReport.java

@@ -5 +5 @@
 import java.io.Serializable;
 import java.io.StringWriter;
+import java.time.Instant;
+import java.util.ArrayDeque;
+import java.util.Deque;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Predicate;
+
+import org.openstreetmap.josm.tools.Pair;
 
 /**
@@ -40 +45 @@
 public final class BugReport implements Serializable {
     private static final long serialVersionUID = 1L;
+    /** The maximum suppressed exceptions to keep to report */
+    private static final byte MAXIMUM_SUPPRESSED_EXCEPTIONS = 4;
+    /** The list of suppressed exceptions, Pair<time reported, exception> */
+    private static final Deque<Pair<Instant, Throwable>> SUPPRESSED_EXCEPTIONS = new ArrayDeque<>(MAXIMUM_SUPPRESSED_EXCEPTIONS);
 
     private boolean includeStatusReport = true;
@@ -54 +63 @@
         this.exception = e;
         includeAllStackTraces = e.mayHaveConcurrentSource();
+    }
+
+    /**
+     * Add a suppressed exception. Mostly useful for when a chain of exceptions causes an actual bug report.
+     * This should only be used when an exception is raised in {@link org.openstreetmap.josm.gui.util.GuiHelper}
+     * or {@link org.openstreetmap.josm.gui.progress.swing.ProgressMonitorExecutor} at this time.
+     * {@link org.openstreetmap.josm.tools.Logging} may call this in the future, when logging a throwable.
+     * @param t The throwable raised. If {@code null}, we add a new {@code NullPointerException} instead.
+     * @since 18549
+     */
+    public static void addSuppressedException(Throwable t) {
+        SUPPRESSED_EXCEPTIONS.add(new Pair<>(Instant.now(), t != null ? t : new NullPointerException()));
+        // Ensure we don't call pop in more than MAXIMUM_SUPPRESSED_EXCEPTIONS threads. This guard is
+        // here just in case someone doesn't read the javadocs.
+        synchronized (SUPPRESSED_EXCEPTIONS) {
+            // Ensure we aren't keeping exceptions forever
+            while (SUPPRESSED_EXCEPTIONS.size() > MAXIMUM_SUPPRESSED_EXCEPTIONS) {
+                SUPPRESSED_EXCEPTIONS.pop();
+            }
+        }
     }
 
@@ -136 +165 @@
             exception.printReportThreadsTo(out);
         }
+        synchronized (SUPPRESSED_EXCEPTIONS) {
+            if (!SUPPRESSED_EXCEPTIONS.isEmpty()) {
+                out.println("=== ADDITIONAL EXCEPTIONS ===");
+                // Avoid multiple bug reports from reading from the deque at the same time.
+                while (SUPPRESSED_EXCEPTIONS.peek() != null) {
+                    Pair<Instant, Throwable> currentException = SUPPRESSED_EXCEPTIONS.pop();
+                    out.println("==== Exception at " + currentException.a.toEpochMilli() + " ====");
+                    currentException.b.printStackTrace(out);
+                }
+            }
+        }
         return stringWriter.toString().replaceAll("\r", "");
     }