Ticket #12905: patch-fix-12905.patch

new file src/org/openstreetmap/josm/tools/StreamUtils.java

@@ +1 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.tools;
+
+import java.util.Iterator;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Utility methods for streams.
+ * @author Michael Zangl
+ * @since xxx
+ */
+public final class StreamUtils {
+
+    /**
+     * Untility class
+     */
+    private StreamUtils() {}
+
+    /**
+     * Convert an iterator to a stream.
+     * @param <T> The element type to iterate over
+     * @param iterator The iterator
+     * @return The stream of for that iterator.
+     */
+    public static <T> Stream<T> toStream(Iterator<? extends T> iterator) {
+        Spliterator<T> spliterator = Spliterators.spliteratorUnknownSize(iterator, Spliterator.ORDERED);
+        return StreamSupport.stream(spliterator, false);
+    }
+}

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

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.tools.bugreport;
 
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+import org.openstreetmap.josm.actions.ShowStatusReportAction;
+
 /**
  * This class contains utility methods to create and handle a bug report.
  * <p>
@@ -19 +25 @@
  * String tag = "...";
  * try {
  *   ... your code ...
- * } catch (Throwable t) {
+ * } catch (RuntimeException t) {
  *   throw BugReport.intercept(t).put("id", id).put("tag", tag);
  * }
  * </pre>
@@ -31 +37 @@
  * @since 10285
  */
 public final class BugReport {
+    private boolean includeStatusReport = true;
+    private boolean includeData = true;
+    private boolean includeAllStackTraces;
+    private ReportedException exception;
+    private final CopyOnWriteArrayList<BugReportListener> listeners = new CopyOnWriteArrayList<>();
+
     /**
      * Create a new bug report
      * @param e The {@link ReportedException} to use. No more data should be added after creating the report.
      */
-    private BugReport(ReportedException e) {
-        // TODO: Use this class to create the bug report.
+    BugReport(ReportedException e) {
+        this.exception = e;
+        includeAllStackTraces = e.mayHaveConcurrentSource();
+    }
+
+    /**
+     * Get if this report should include a system status report
+     * @return <code>true</code> to include it.
+     * @since xxx
+     */
+    public boolean getIncludeStatusReport() {
+        return includeStatusReport;
+    }
+
+    /**
+     * Set if this report should include a system status report
+     * @param includeStatusReport if the status report should be included
+     * @since xxx
+     */
+    public void setIncludeStatusReport(boolean includeStatusReport) {
+        this.includeStatusReport = includeStatusReport;
+        fireChange();
+    }
+
+    /**
+     * Get if this report should include the data that was traced.
+     * @return <code>true</code> to include it.
+     * @since xxx
+     */
+    public boolean getIncludeData() {
+        return includeData;
+    }
+
+    /**
+     * Set if this report should include the data that was traced.
+     * @param includeData if data should be included
+     * @since xxx
+     */
+    public void setIncludeData(boolean includeData) {
+        this.includeData = includeData;
+        fireChange();
+    }
+
+    /**
+     * Get if this report should include the stack traces for all other threads.
+     * @return <code>true</code> to include it.
+     * @since xxx
+     */
+    public boolean getIncludeAllStackTraces() {
+        return includeAllStackTraces;
+    }
+
+    /**
+     * Sets if this report should include the stack traces for all other threads.
+     * @param includeAllStackTraces if all stack traces should be included
+     * @since xxx
+     */
+    public void setIncludeAllStackTraces(boolean includeAllStackTraces) {
+        this.includeAllStackTraces = includeAllStackTraces;
+        fireChange();
+    }
+
+    /**
+     * Gets the full string that should be send as error report.
+     * @return The string.
+     * @since xxx
+     */
+    public String getReportText() {
+        StringWriter stringWriter = new StringWriter();
+        PrintWriter out = new PrintWriter(stringWriter);
+        if (getIncludeStatusReport()) {
+            out.println(ShowStatusReportAction.getReportHeader());
+        }
+        if (getIncludeData()) {
+            exception.printReportDataTo(out);
+        }
+        exception.printReportStackTo(out);
+        if (getIncludeAllStackTraces()) {
+            exception.printReportThreadsTo(out);
+        }
+        return stringWriter.toString().replaceAll("\r", "");
+    }
+
+    /**
+     * Add a new change listener.
+     * @param listener The listener
+     * @since xxx
+     */
+    public void addChangeListener(BugReportListener listener) {
+        listeners.add(listener);
+    }
+
+    /**
+     * Remove a change listener.
+     * @param listener The listener
+     * @since xxx
+     */
+    public void removeChangeListener(BugReportListener listener) {
+        listeners.remove(listener);
+    }
+
+    private void fireChange() {
+        listeners.stream().forEach(l -> l.bugReportChanged(this));
     }
 
     /**
@@ -74 +187 @@
         }
         return "?";
     }
+
+    /**
+     * A listener that listens to changes to this report.
+     * @author Michael Zangl
+     * @since xxx
+     */
+    @FunctionalInterface
+    public interface BugReportListener {
+        /**
+         * Called whenever this bug report was changed, e.g. the data to be included in it.
+         * @param report The report that was changed.
+         */
+        void bugReportChanged(BugReport report);
+    }
 }

src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java

@@ -196 +196 @@
     }
 
     static JPanel buildPanel(final Throwable e) {
-        StringWriter stack = new StringWriter();
-        PrintWriter writer = new PrintWriter(stack);
+        DebugTextDisplay textarea;
         if (e instanceof ReportedException) {
             // Temporary!
-            ((ReportedException) e).printReportDataTo(writer);
-            ((ReportedException) e).printReportStackTo(writer);
+            textarea = new DebugTextDisplay(new BugReport((ReportedException) e));
         } else {
+            StringWriter stack = new StringWriter();
+            PrintWriter writer = new PrintWriter(stack);
             e.printStackTrace(writer);
+            String text = ShowStatusReportAction.getReportHeader() + stack.getBuffer().toString();
+            textarea = new DebugTextDisplay(text);
         }
 
-        String text = ShowStatusReportAction.getReportHeader() + stack.getBuffer().toString();
-        text = text.replaceAll("\r", "");
-
         JPanel p = new JPanel(new GridBagLayout());
         p.add(new JMultilineLabel(
                 tr("You have encountered an error in JOSM. Before you file a bug report " +
@@ -219 +218 @@
                 tr("You should also update your plugins. If neither of those help please " +
                         "file a bug report in our bugtracker using this link:")),
                         GBC.eol().fill(GridBagConstraints.HORIZONTAL));
-        p.add(new JButton(new ReportBugAction(text)), GBC.eop().insets(8, 0, 0, 0));
+        p.add(new JButton(new ReportBugAction(textarea.getCodeText())), GBC.eop().insets(8, 0, 0, 0));
         p.add(new JMultilineLabel(
                 tr("There the error information provided below should already be " +
                         "filled in for you. Please include information on how to reproduce " +
@@ -230 +229 @@
                         "below at this URL:")), GBC.eol().fill(GridBagConstraints.HORIZONTAL));
         p.add(new UrlLabel(Main.getJOSMWebsite()+"/newticket", 2), GBC.eop().insets(8, 0, 0, 0));
 
-        // Wiki formatting for manual copy-paste
-        DebugTextDisplay textarea = new DebugTextDisplay(text);
-
         if (textarea.copyToClippboard()) {
             p.add(new JLabel(tr("(The text has already been copied to your clipboard.)")),
                     GBC.eop().fill(GridBagConstraints.HORIZONTAL));

new file src/org/openstreetmap/josm/tools/bugreport/BugReportSettingsPanel.java

@@ +1 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.tools.bugreport;
+
+import static org.openstreetmap.josm.tools.I18n.tr;
+
+import javax.swing.BoxLayout;
+import javax.swing.JCheckBox;
+import javax.swing.JPanel;
+
+/**
+ * This panel displays the settings that can be changed before submitting a bug report to the web page.
+ * @author Michael Zangl
+ * @since xxx
+ */
+public class BugReportSettingsPanel extends JPanel {
+    /**
+     * Creates the new settings panel.
+     * @param report The report this panel should influence.
+     */
+    public BugReportSettingsPanel(BugReport report) {
+        setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));
+
+        JCheckBox statusReport = new JCheckBox(tr("Include the system status report."));
+        statusReport.setSelected(report.getIncludeStatusReport());
+        statusReport.addChangeListener(e -> report.setIncludeStatusReport(statusReport.isSelected()));
+        add(statusReport);
+
+        JCheckBox data = new JCheckBox(tr("Include information about the data that was worked on."));
+        data.setSelected(report.getIncludeData());
+        data.addChangeListener(e -> report.setIncludeData(data.isSelected()));
+        add(data);
+
+        JCheckBox allStackTraces = new JCheckBox(tr("Include all stack traces."));
+        allStackTraces.setSelected(report.getIncludeAllStackTraces());
+        allStackTraces.addChangeListener(e -> report.setIncludeAllStackTraces(allStackTraces.isSelected()));
+        add(allStackTraces);
+    }
+}

src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java

@@ -14 +14 @@
  * @since 10055
  */
 public class DebugTextDisplay extends JScrollPane {
-    private final String text;
+    private static final String CODE_PATTERN = "{{{%n%s%n}}}";
+    private String text;
+    private JosmTextArea textArea;
 
     /**
-     * Creates a new text are with the fixed text
-     * @param textToDisplay The text to display.
+     * Creates a new text area.
+     * @since xxx
      */
-    public DebugTextDisplay(String textToDisplay) {
-        text = "{{{\n" + Utils.strip(textToDisplay) + "\n}}}";
-        JosmTextArea textArea = new JosmTextArea(text);
+    private DebugTextDisplay() {
+        textArea = new JosmTextArea();
         textArea.setCaretPosition(0);
         textArea.setEditable(false);
         setViewportView(textArea);
@@ -30 +31 @@
     }
 
     /**
-     * Copies the debug text to the clippboard.
+     * Creates a new text area with an inital text to display
+     * @param textToDisplay The text to display.
+     */
+    public DebugTextDisplay(String textToDisplay) {
+        this();
+        setCodeText(textToDisplay);
+    }
+
+    /**
+     * Creates a new text area that displays the bug report data
+     * @param report The bug report data to display.
+     */
+    public DebugTextDisplay(BugReport report) {
+        this();
+        setCodeText(report.getReportText());
+        report.addChangeListener(e -> setCodeText(report.getReportText()));
+    }
+
+    /**
+     * Sets the text that should be displayed in this view.
+     * @param textToDisplay The text
+     * @since xxx
+     */
+    private void setCodeText(String textToDisplay) {
+        text = Utils.strip(textToDisplay).replaceAll("\r", "");
+        textArea.setText(String.format(CODE_PATTERN, text));
+    }
+
+    /**
+     * Copies the debug text to the clippboard. This includes the code tags for trac.
      * @return <code>true</code> if copy was successful
+     * @since 10055
      */
     public boolean copyToClippboard() {
-        return Utils.copyToClipboard(text);
+        return Utils.copyToClipboard(String.format(CODE_PATTERN, text));
+    }
+
+    /**
+     * Gets the text this are displays, without the code tag.
+     * @return The stripped text set by {@link #setCodeText(String)}
+     * @since xxx
+     */
+    public String getCodeText() {
+        return text;
     }
 }

src/org/openstreetmap/josm/tools/bugreport/ReportedException.java

@@ -2 +2 @@
 package org.openstreetmap.josm.tools.bugreport;
 
 import java.io.PrintWriter;
+import java.io.Serializable;
+import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.ConcurrentModificationException;
 import java.util.IdentityHashMap;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.NoSuchElementException;
 import java.util.Set;
 
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.tools.StreamUtils;
 
 /**
  * This is a special exception that cannot be directly thrown.
@@ -24 +30 @@
  * @since 10285
  */
 public class ReportedException extends RuntimeException {
-    private static final int MAX_COLLECTION_ENTRIES = 30;
     /**
-     *
+     * How many entries of a collection to include in the bug report.
      */
+    private static final int MAX_COLLECTION_ENTRIES = 30;
+
     private static final long serialVersionUID = 737333873766201033L;
+
     /**
      * We capture all stack traces on exception creation. This allows us to trace synchonization problems better. We cannot be really sure what
      * happened but we at least see which threads
      */
     private final transient Map<Thread, StackTraceElement[]> allStackTraces;
-    private final transient LinkedList<Section> sections = new LinkedList<>();
+    private final LinkedList<Section> sections = new LinkedList<>();
     private final transient Thread caughtOnThread;
     private final Throwable exception;
     private String methodWarningFrom;
@@ -143 +151 @@
             return false;
         }
 
-        Set<Throwable> dejaVu = Collections.newSetFromMap(new IdentityHashMap<Throwable, Boolean>());
-        return hasSameStackTrace(dejaVu, this.exception, e.exception);
+        return hasSameStackTrace(new CauseTraceIterator(), e.exception);
     }
 
-    private static boolean hasSameStackTrace(Set<Throwable> dejaVu, Throwable e1, Throwable e2) {
-        if (dejaVu.contains(e1)) {
-            // cycle. If it was the same until here, we assume both have that cycle.
+    private static boolean hasSameStackTrace(CauseTraceIterator causeTraceIterator, Throwable e2) {
+        if (!causeTraceIterator.hasNext()) {
+            // all done.
             return true;
         }
-        dejaVu.add(e1);
-
+        Throwable e1 = causeTraceIterator.next();
         StackTraceElement[] t1 = e1.getStackTrace();
         StackTraceElement[] t2 = e2.getStackTrace();
 
@@ -166 +172 @@
         if ((c1 == null) != (c2 == null)) {
             return false;
         } else if (c1 != null) {
-            return hasSameStackTrace(dejaVu, c1, c2);
+            return hasSameStackTrace(causeTraceIterator, c2);
         } else {
             return true;
         }
@@ -227 +233 @@
             .toString();
     }
 
-    private static class SectionEntry {
+
+    /**
+     * Check if this exception may be caused by a threading issue.
+     * @return <code>true</code> if it is.
+     * @since xxx
+     */
+    public boolean mayHaveConcurrentSource() {
+        return StreamUtils.toStream(new CauseTraceIterator())
+                .anyMatch(t -> t instanceof ConcurrentModificationException || t instanceof InvocationTargetException);
+    }
+
+    /**
+     * Iterates over the causes for this exception. Ignores cycles and aborts iteration then.
+     * @author Michal Zangl
+     * @since xxx
+     */
+    private final class CauseTraceIterator implements Iterator<Throwable> {
+        private Throwable current = exception;
+        private final Set<Throwable> dejaVu = Collections.newSetFromMap(new IdentityHashMap<Throwable, Boolean>());
+
+        @Override
+        public boolean hasNext() {
+            return current != null;
+        }
+
+        @Override
+        public Throwable next() {
+            if (!hasNext()) {
+                throw new NoSuchElementException();
+            }
+            Throwable toReturn = current;
+            advance();
+            return toReturn;
+        }
+
+        private void advance() {
+            dejaVu.add(current);
+            current = current.getCause();
+            if (current != null && dejaVu.contains(current)) {
+                current = null;
+            }
+        }
+    }
+
+    private static class SectionEntry implements Serializable {
+
+        private static final long serialVersionUID = 1L;
+
         private final String key;
         private final String value;
 
@@ -248 +301 @@
         }
     }
 
-    private static class Section {
+    private static class Section implements Serializable {
+
+        private static final long serialVersionUID = 1L;
 
         private final String sectionName;
         private final ArrayList<SectionEntry> entries = new ArrayList<>();