From 35848e09a83a0bd74e7dae1d2ddeee3b6b2d9ddf Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 10 Dec 2014 10:26:02 +0100 Subject: [PATCH 1/4] Mitigated Serial Monitor resource exhaustion when the connected device sends a lot of data Fixes #2233 --- app/src/processing/app/SerialMonitor.java | 52 +++++++++---- .../processing/app/debug/TextAreaFIFO.java | 78 +++++++++++++++++++ build/shared/revisions.txt | 3 + 3 files changed, 120 insertions(+), 13 deletions(-) create mode 100644 app/src/processing/app/debug/TextAreaFIFO.java diff --git a/app/src/processing/app/SerialMonitor.java b/app/src/processing/app/SerialMonitor.java index 1f34e8f7e55..10a23331ee0 100644 --- a/app/src/processing/app/SerialMonitor.java +++ b/app/src/processing/app/SerialMonitor.java @@ -19,6 +19,7 @@ package processing.app; import processing.app.debug.MessageConsumer; +import processing.app.debug.TextAreaFIFO; import processing.core.*; import static processing.app.I18n._; @@ -26,13 +27,12 @@ import java.awt.event.*; import javax.swing.*; import javax.swing.border.*; -import javax.swing.event.*; import javax.swing.text.*; -public class SerialMonitor extends JFrame implements MessageConsumer { +public class SerialMonitor extends JFrame implements MessageConsumer,ActionListener { private Serial serial; private String port; - private JTextArea textArea; + private TextAreaFIFO textArea; private JScrollPane scrollPane; private JTextField textField; private JButton sendButton; @@ -40,6 +40,8 @@ public class SerialMonitor extends JFrame implements MessageConsumer { private JComboBox lineEndings; private JComboBox serialRates; private int serialRate; + private javax.swing.Timer updateTimer; + private StringBuffer updateBuffer; public SerialMonitor(String port) { super(port); @@ -67,7 +69,9 @@ public void actionPerformed(ActionEvent e) { Font editorFont = Preferences.getFont("editor.font"); Font font = new Font(consoleFont.getName(), consoleFont.getStyle(), editorFont.getSize()); - textArea = new JTextArea(16, 40); + textArea = new TextAreaFIFO(4000000); + textArea.setRows(16); + textArea.setColumns(40); textArea.setEditable(false); textArea.setFont(font); @@ -171,6 +175,9 @@ public void actionPerformed(ActionEvent event) { } } } + + updateBuffer = new StringBuffer(1048576); + updateTimer = new javax.swing.Timer(33, this); // redraw serial monitor at 30 Hz } protected void setPlacement(int[] location) { @@ -203,9 +210,9 @@ private void send(String s) { public void openSerialPort() throws SerialException { if (serial != null) return; - serial = new Serial(port, serialRate); serial.addListener(this); + updateTimer.start(); } public void closeSerialPort() { @@ -219,13 +226,32 @@ public void closeSerialPort() { } } - public void message(final String s) { - SwingUtilities.invokeLater(new Runnable() { - public void run() { - textArea.append(s); - if (autoscrollBox.isSelected()) { - textArea.setCaretPosition(textArea.getDocument().getLength()); - } - }}); + public void message(String s) { + // TODO: can we pass a byte array, to avoid overhead of String + addToUpdateBuffer(s); } + + private synchronized void addToUpdateBuffer(String s) { + updateBuffer.append(s); + } + + private synchronized String consumeUpdateBuffer() { + String s = updateBuffer.toString(); + updateBuffer.setLength(0); + return s; + } + + public void actionPerformed(ActionEvent e) { + final String s = consumeUpdateBuffer(); + if (s.length() > 0) { + //System.out.println("gui append " + s.length()); + boolean scroll = autoscrollBox.isSelected(); + textArea.allowTrim(scroll); + textArea.append(s); + if (scroll) { + textArea.setCaretPosition(textArea.getDocument().getLength()); + } + } + } + } diff --git a/app/src/processing/app/debug/TextAreaFIFO.java b/app/src/processing/app/debug/TextAreaFIFO.java new file mode 100644 index 00000000000..9c5dc8612ad --- /dev/null +++ b/app/src/processing/app/debug/TextAreaFIFO.java @@ -0,0 +1,78 @@ +/* + Copyright (c) 2014 Paul Stoffregen + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software Foundation, + Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + + $Id$ +*/ + +// adapted from https://community.oracle.com/thread/1479784 + +package processing.app.debug; + +import javax.swing.JTextArea; +import javax.swing.SwingUtilities; +import javax.swing.event.DocumentEvent; +import javax.swing.event.DocumentListener; +import javax.swing.text.BadLocationException; + +public class TextAreaFIFO extends JTextArea implements DocumentListener { + private int maxChars; + + private int updateCount; // limit how often we trim the document + + private boolean doTrim; + + public TextAreaFIFO(int max) { + maxChars = max; + updateCount = 0; + doTrim = true; + getDocument().addDocumentListener(this); + } + + public void allowTrim(boolean trim) { + doTrim = trim; + } + + public void insertUpdate(DocumentEvent e) { + if (++updateCount > 150 && doTrim) { + updateCount = 0; + SwingUtilities.invokeLater(new Runnable() { + public void run() { + trimDocument(); + } + }); + } + } + + public void removeUpdate(DocumentEvent e) { + } + + public void changedUpdate(DocumentEvent e) { + } + + public void trimDocument() { + int len = 0; + len = getDocument().getLength(); + if (len > maxChars) { + int n = len - maxChars; + System.out.println("trimDocument: remove " + n + " chars"); + try { + getDocument().remove(0, n); + } catch (BadLocationException ble) { + } + } + } +} diff --git a/build/shared/revisions.txt b/build/shared/revisions.txt index e960fd3725f..c55aec50a1c 100644 --- a/build/shared/revisions.txt +++ b/build/shared/revisions.txt @@ -13,6 +13,9 @@ ARDUINO 1.0.7 * Fixed missing NOT_AN_INTERRUPT constant in digitalPinToInterrupt() macro * Fixed performance regression in HardwareSerial::available() introduced with https://github.com/arduino/Arduino/pull/2057 +[ide] +* Mitigated Serial Monitor resource exhaustion when the connected device sends a lot of data (Paul Stoffregen) + ARDUINO 1.0.6 - 2014.09.16 [core] From 391d3380ee37cba0b99d24e4f4aef558f2065480 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 10 Dec 2014 11:01:45 +0100 Subject: [PATCH 2/4] Removed leftover debug print --- app/src/processing/app/debug/TextAreaFIFO.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/processing/app/debug/TextAreaFIFO.java b/app/src/processing/app/debug/TextAreaFIFO.java index 9c5dc8612ad..9a6d575e176 100644 --- a/app/src/processing/app/debug/TextAreaFIFO.java +++ b/app/src/processing/app/debug/TextAreaFIFO.java @@ -68,7 +68,7 @@ public void trimDocument() { len = getDocument().getLength(); if (len > maxChars) { int n = len - maxChars; - System.out.println("trimDocument: remove " + n + " chars"); + //System.out.println("trimDocument: remove " + n + " chars"); try { getDocument().remove(0, n); } catch (BadLocationException ble) { From 63f5d26ae9186419ae65a36bbced33f94a2fdfd9 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 23 Dec 2014 12:47:24 +0100 Subject: [PATCH 3/4] Improved Serial input processing. Before this patch every byte received from Serial invokes a String allocation, not really efficient. Moreover a InputStreamReader is chained on the serial InputStream to correctly convert bytes into UTF-8 characters. --- app/src/processing/app/Serial.java | 57 +++++++---------------- app/src/processing/app/SerialMonitor.java | 21 ++++----- 2 files changed, 26 insertions(+), 52 deletions(-) diff --git a/app/src/processing/app/Serial.java b/app/src/processing/app/Serial.java index 14c0933c0a6..b9432f9043f 100755 --- a/app/src/processing/app/Serial.java +++ b/app/src/processing/app/Serial.java @@ -25,9 +25,7 @@ package processing.app; //import processing.core.*; -import processing.app.debug.MessageConsumer; import static processing.app.I18n._; - import gnu.io.*; import java.io.*; @@ -55,15 +53,13 @@ public class Serial implements SerialPortEventListener { // read buffer and streams - InputStream input; + InputStreamReader input; OutputStream output; byte buffer[] = new byte[32768]; int bufferIndex; int bufferLast; - MessageConsumer consumer; - public Serial(boolean monitor) throws SerialException { this(Preferences.get("serial.port"), Preferences.getInteger("serial.debug_rate"), @@ -158,7 +154,7 @@ public Serial(String iname, int irate, if (portId.getName().equals(iname)) { //System.out.println("looking for "+iname); port = (SerialPort)portId.open("serial madness", 2000); - input = port.getInputStream(); + input = new InputStreamReader(port.getInputStream()); output = port.getOutputStream(); port.setSerialPortParams(rate, databits, stopbits, parity); port.addEventListener(this); @@ -237,62 +233,41 @@ public void dispose() { port = null; } + char serialBuffer[] = new char[4096]; - public void addListener(MessageConsumer consumer) { - this.consumer = consumer; - } - - synchronized public void serialEvent(SerialPortEvent serialEvent) { - //System.out.println("serial port event"); // " + serialEvent); - //System.out.flush(); - //System.out.println("into"); - //System.out.flush(); - //System.err.println("type " + serialEvent.getEventType()); - //System.err.println("ahoooyey"); - //System.err.println("ahoooyeysdfsdfsdf"); if (serialEvent.getEventType() == SerialPortEvent.DATA_AVAILABLE) { - //System.out.println("data available"); - //System.err.flush(); try { - while (input.available() > 0) { - //if (input.available() > 0) { - //serial = input.read(); - //serialEvent(); - //buffer[bufferCount++] = (byte) serial; + while (input.ready()) { synchronized (buffer) { if (bufferLast == buffer.length) { byte temp[] = new byte[bufferLast << 1]; System.arraycopy(buffer, 0, temp, 0, bufferLast); buffer = temp; } - //buffer[bufferLast++] = (byte) input.read(); - if(monitor == true) - System.out.print((char) input.read()); - if (this.consumer != null) - this.consumer.message("" + (char) input.read()); - - /* - System.err.println(input.available() + " " + - ((char) buffer[bufferLast-1])); - */ //} + int n = input.read(serialBuffer); + message(serialBuffer, n); } } - //System.out.println("no more"); - } catch (IOException e) { errorMessage("serialEvent", e); - //e.printStackTrace(); - //System.out.println("angry"); } catch (Exception e) { } } - //System.out.println("out of"); - //System.err.println("out of event " + serialEvent.getEventType()); } + /** + * This method is intended to be redefined by users of Serial class + * + * @param buff + * @param n + */ + protected void message(char buff[], int n) { + // Empty + } + /** * Returns the number of bytes that have been read from serial * and are waiting to be dealt with by the user. diff --git a/app/src/processing/app/SerialMonitor.java b/app/src/processing/app/SerialMonitor.java index 10a23331ee0..64c8bb399ed 100644 --- a/app/src/processing/app/SerialMonitor.java +++ b/app/src/processing/app/SerialMonitor.java @@ -18,18 +18,18 @@ package processing.app; -import processing.app.debug.MessageConsumer; import processing.app.debug.TextAreaFIFO; import processing.core.*; import static processing.app.I18n._; import java.awt.*; import java.awt.event.*; + import javax.swing.*; import javax.swing.border.*; import javax.swing.text.*; -public class SerialMonitor extends JFrame implements MessageConsumer,ActionListener { +public class SerialMonitor extends JFrame implements ActionListener { private Serial serial; private String port; private TextAreaFIFO textArea; @@ -210,8 +210,12 @@ private void send(String s) { public void openSerialPort() throws SerialException { if (serial != null) return; - serial = new Serial(port, serialRate); - serial.addListener(this); + serial = new Serial(port, serialRate) { + @Override + protected void message(char buff[], int n) { + addToUpdateBuffer(buff, n); + } + }; updateTimer.start(); } @@ -226,13 +230,8 @@ public void closeSerialPort() { } } - public void message(String s) { - // TODO: can we pass a byte array, to avoid overhead of String - addToUpdateBuffer(s); - } - - private synchronized void addToUpdateBuffer(String s) { - updateBuffer.append(s); + private synchronized void addToUpdateBuffer(char buff[], int n) { + updateBuffer.append(buff, 0, n); } private synchronized String consumeUpdateBuffer() { From 8e0a311e871a3feb505f18771a6fd58abf5048cd Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 23 Dec 2014 14:06:23 +0100 Subject: [PATCH 4/4] SerialMonitor: limit buffering without autoscroll When the "autoscroll" checkbox is deselected the buffer may continue to grow up to twice of the maximum size. This is a compromise to ensure a better user experience and, at the same time, reduce the chance to lose data and get "holes" in the serial stream. See #2491 --- app/src/processing/app/SerialMonitor.java | 10 +++---- .../processing/app/debug/TextAreaFIFO.java | 26 ++++++++++++++----- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/app/src/processing/app/SerialMonitor.java b/app/src/processing/app/SerialMonitor.java index 64c8bb399ed..7ce00474bb5 100644 --- a/app/src/processing/app/SerialMonitor.java +++ b/app/src/processing/app/SerialMonitor.java @@ -69,7 +69,7 @@ public void actionPerformed(ActionEvent e) { Font editorFont = Preferences.getFont("editor.font"); Font font = new Font(consoleFont.getName(), consoleFont.getStyle(), editorFont.getSize()); - textArea = new TextAreaFIFO(4000000); + textArea = new TextAreaFIFO(8000000); textArea.setRows(16); textArea.setColumns(40); textArea.setEditable(false); @@ -244,11 +244,11 @@ public void actionPerformed(ActionEvent e) { final String s = consumeUpdateBuffer(); if (s.length() > 0) { //System.out.println("gui append " + s.length()); - boolean scroll = autoscrollBox.isSelected(); - textArea.allowTrim(scroll); - textArea.append(s); - if (scroll) { + if (autoscrollBox.isSelected()) { + textArea.appendTrim(s); textArea.setCaretPosition(textArea.getDocument().getLength()); + } else { + textArea.appendNoTrim(s); } } } diff --git a/app/src/processing/app/debug/TextAreaFIFO.java b/app/src/processing/app/debug/TextAreaFIFO.java index 9a6d575e176..9783cd42c17 100644 --- a/app/src/processing/app/debug/TextAreaFIFO.java +++ b/app/src/processing/app/debug/TextAreaFIFO.java @@ -30,6 +30,7 @@ public class TextAreaFIFO extends JTextArea implements DocumentListener { private int maxChars; + private int trimMaxChars; private int updateCount; // limit how often we trim the document @@ -37,15 +38,12 @@ public class TextAreaFIFO extends JTextArea implements DocumentListener { public TextAreaFIFO(int max) { maxChars = max; + trimMaxChars = max / 2; updateCount = 0; doTrim = true; getDocument().addDocumentListener(this); } - public void allowTrim(boolean trim) { - doTrim = trim; - } - public void insertUpdate(DocumentEvent e) { if (++updateCount > 150 && doTrim) { updateCount = 0; @@ -66,8 +64,8 @@ public void changedUpdate(DocumentEvent e) { public void trimDocument() { int len = 0; len = getDocument().getLength(); - if (len > maxChars) { - int n = len - maxChars; + if (len > trimMaxChars) { + int n = len - trimMaxChars; //System.out.println("trimDocument: remove " + n + " chars"); try { getDocument().remove(0, n); @@ -75,4 +73,20 @@ public void trimDocument() { } } } + + public void appendNoTrim(String s) { + int free = maxChars - getDocument().getLength(); + if (free <= 0) + return; + if (s.length() > free) + append(s.substring(0, free)); + else + append(s); + doTrim = false; + } + + public void appendTrim(String str) { + append(str); + doTrim = true; + } }