From 2a50a19d73ef4b8f5321d4e393262f416b38e5ac Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 29 Jul 2015 20:39:00 -0500 Subject: [PATCH] Avoid pathological failure mode when trimming lines If for any reason the document associated with a LimitLinesDocumentListener ended up not getting shorter when its first line was removed (this is, of course, impossible), sklauncher would wedge in a busy loop that completely killed the GUI thread forever. Solution, part #1: Don't do a while loop, just remove multiple lines if you need to. But wait, why are we having this problem at all? It turns out that there are problems when the document's length changes, which can manifest as tons of BadLocationExceptions going uncaught in the AWT thread which is trying to refresh the window. Why? Probably because the insert into the document isn't happening in the Swing run queue, but concurrently with it. So use an invokeLater to do the actual insertion. With these two changes, and a console set to 125 lines, I got no failures. Previously, if I set the console to 125 lines, it would die catastrophically before Minecraft was done starting up. Also performance may be noticably better. It's pretty common for things to dump 20+ lines into the buffer at once. If you have a 10,000 line buffer, and you then delete the first line of it 20 times, that's actually a whole lot of allocation and shuffling going on. --- .../skcraft/launcher/swing/MessageLog.java | 26 +++++++---- .../util/LimitLinesDocumentListener.java | 46 +++++++++++-------- 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/launcher/src/main/java/com/skcraft/launcher/swing/MessageLog.java b/launcher/src/main/java/com/skcraft/launcher/swing/MessageLog.java index ccc877a..1281540 100644 --- a/launcher/src/main/java/com/skcraft/launcher/swing/MessageLog.java +++ b/launcher/src/main/java/com/skcraft/launcher/swing/MessageLog.java @@ -111,21 +111,29 @@ public class MessageLog extends JPanel { * @param line line * @param attributes attribute set, or null for none */ - public void log(String line, AttributeSet attributes) { + public void log(final String line, AttributeSet attributes) { + final Document d = document; + final JTextComponent t = textComponent; if (colorEnabled) { if (line.startsWith("(!!)")) { attributes = highlightedAttributes; } } + final AttributeSet a = attributes; - try { - int offset = document.getLength(); - document.insertString(offset, line, - (attributes != null && colorEnabled) ? attributes : defaultAttributes); - textComponent.setCaretPosition(document.getLength()); - } catch (BadLocationException ble) { - - } + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + try { + int offset = d.getLength(); + d.insertString(offset, line, + (a != null && colorEnabled) ? a : defaultAttributes); + t.setCaretPosition(d.getLength()); + } catch (BadLocationException ble) { + + } + } + }); } /** diff --git a/launcher/src/main/java/com/skcraft/launcher/util/LimitLinesDocumentListener.java b/launcher/src/main/java/com/skcraft/launcher/util/LimitLinesDocumentListener.java index 92da7b1..6f9c09a 100644 --- a/launcher/src/main/java/com/skcraft/launcher/util/LimitLinesDocumentListener.java +++ b/launcher/src/main/java/com/skcraft/launcher/util/LimitLinesDocumentListener.java @@ -21,6 +21,7 @@ import javax.swing.text.Element; public class LimitLinesDocumentListener implements DocumentListener { private int maximumLines; private boolean isRemoveFromStart; + private volatile boolean isRemoving; /** * Specify the number of lines to be stored in the Document. Extra lines @@ -34,6 +35,7 @@ public class LimitLinesDocumentListener implements DocumentListener { boolean isRemoveFromStart) { setLimitLines(maximumLines); this.isRemoveFromStart = isRemoveFromStart; + this.isRemoving = false; } /** @@ -54,12 +56,15 @@ public class LimitLinesDocumentListener implements DocumentListener { // Changes to the Document can not be done within the listener // so we need to add the processing to the end of the EDT - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - removeLines(e); - } - }); + if (!this.isRemoving) { + this.isRemoving = true; + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + removeLines(e); + } + }); + } } @Override @@ -74,20 +79,25 @@ public class LimitLinesDocumentListener implements DocumentListener { // The root Element of the Document will tell us the total number // of line in the Document. - Document document = e.getDocument(); - Element root = document.getDefaultRootElement(); + try { + Document document = e.getDocument(); + Element root = document.getDefaultRootElement(); + int excess = root.getElementCount() - maximumLines; - while (root.getElementCount() > maximumLines) { - if (isRemoveFromStart) { - removeFromStart(document, root); - } else { - removeFromEnd(document, root); + if (excess > 0) { + if (isRemoveFromStart) { + removeFromStart(document, root, excess); + } else { + removeFromEnd(document, root); + } } + } finally { + this.isRemoving = false; } } - private void removeFromStart(Document document, Element root) { - Element line = root.getElement(0); + private void removeFromStart(Document document, Element root, int excess) { + Element line = root.getElement(excess - 1); int end = line.getEndOffset(); try { @@ -101,9 +111,9 @@ public class LimitLinesDocumentListener implements DocumentListener { // We use start minus 1 to make sure we remove the newline // character of the previous line - Element line = root.getElement(root.getElementCount() - 1); + Element line = root.getElement(maximumLines); int start = line.getStartOffset(); - int end = line.getEndOffset(); + int end = root.getEndOffset(); try { document.remove(start - 1, end - start); @@ -111,4 +121,4 @@ public class LimitLinesDocumentListener implements DocumentListener { System.out.println(ble); } } -} \ No newline at end of file +}