Bug 558489 - [console] Remove unnecessary recoding in ProcessConsole
output handling
Apart from removing an unnecessary string -> bytes -> string round trip
it fix a potential content corruption since the source (TextConsole)
encoding and target (IOConsoleOutputStream) encoding can differ.
Also the stream listener implementation in ProcessConsole is simplified
a lot and lost some dead code.
Last but not least it replaced some usages of encoding names as strings
with using the Charset class to reduce overall number of charset
lookups.
Change-Id: Ie94aa433e571a2f9898c950d2997f598618aca18
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>
diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java b/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java
index e731732..8ee0a09 100644
--- a/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java
+++ b/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java
@@ -14,6 +14,9 @@
package org.eclipse.debug.core.model;
+import java.nio.charset.Charset;
+import java.nio.charset.IllegalCharsetNameException;
+import java.nio.charset.UnsupportedCharsetException;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
@@ -281,7 +284,15 @@
return new NullStreamsProxy(getSystemProcess());
}
String encoding = getLaunch().getAttribute(DebugPlugin.ATTR_CONSOLE_ENCODING);
- return new StreamsProxy(getSystemProcess(), encoding);
+ Charset charset = null;
+ if (encoding != null) {
+ try {
+ charset = Charset.forName(encoding);
+ } catch (UnsupportedCharsetException | IllegalCharsetNameException e) {
+ DebugPlugin.log(e);
+ }
+ }
+ return new StreamsProxy(getSystemProcess(), charset);
}
/**
diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/InputStreamMonitor.java b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/InputStreamMonitor.java
index 199b183..94e2fb7 100644
--- a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/InputStreamMonitor.java
+++ b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/InputStreamMonitor.java
@@ -16,6 +16,7 @@
import java.io.IOException;
import java.io.OutputStream;
+import java.nio.charset.Charset;
import java.util.Vector;
import org.eclipse.debug.core.DebugPlugin;
@@ -52,9 +53,9 @@
private boolean fClosed = false;
/**
- * The encoding of the input stream.
+ * The charset of the input stream.
*/
- private String fEncoding;
+ private Charset fCharset;
/**
* Creates an input stream monitor which writes to system in via the given output stream.
@@ -66,16 +67,17 @@
}
/**
- * Creates an input stream monitor which writes to system in via the given output stream.
+ * Creates an input stream monitor which writes to system in via the given
+ * output stream.
*
* @param stream output stream
- * @param encoding stream encoding or <code>null</code> for system default
+ * @param charset stream charset or <code>null</code> for system default
*/
- public InputStreamMonitor(OutputStream stream, String encoding) {
- fStream= stream;
+ public InputStreamMonitor(OutputStream stream, Charset charset) {
+ fStream = stream;
fQueue = new Vector<>();
- fLock= new Object();
- fEncoding= encoding;
+ fLock = new Object();
+ fCharset = charset;
}
/**
@@ -139,8 +141,8 @@
String text = fQueue.firstElement();
fQueue.removeElementAt(0);
try {
- if (fEncoding != null) {
- fStream.write(text.getBytes(fEncoding));
+ if (fCharset != null) {
+ fStream.write(text.getBytes(fCharset));
} else {
fStream.write(text.getBytes());
}
diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/OutputStreamMonitor.java b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/OutputStreamMonitor.java
index 34aa133..00daec4 100644
--- a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/OutputStreamMonitor.java
+++ b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/OutputStreamMonitor.java
@@ -18,6 +18,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
+import java.nio.charset.Charset;
import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.core.runtime.ISafeRunnable;
@@ -74,20 +75,20 @@
private long lastSleep;
- private String fEncoding;
+ private Charset fCharset;
private final AtomicBoolean fDone;
/**
- * Creates an output stream monitor on the
- * given stream (connected to system out or err).
+ * Creates an output stream monitor on the given stream (connected to system
+ * out or err).
*
* @param stream input stream to read from
- * @param encoding stream encoding or <code>null</code> for system default
+ * @param charset stream charset or <code>null</code> for system default
*/
- public OutputStreamMonitor(InputStream stream, String encoding) {
+ public OutputStreamMonitor(InputStream stream, Charset charset) {
fStream = new BufferedInputStream(stream, 8192);
- fEncoding = encoding;
+ fCharset = charset;
fContents= new StringBuilder();
fDone = new AtomicBoolean(false);
}
@@ -149,7 +150,7 @@
long currentTime = lastSleep;
char[] chars = new char[BUFFER_SIZE];
int read = 0;
- try (InputStreamReader reader = (fEncoding == null ? new InputStreamReader(fStream) : new InputStreamReader(fStream, fEncoding))) {
+ try (InputStreamReader reader = (fCharset == null ? new InputStreamReader(fStream) : new InputStreamReader(fStream, fCharset))) {
while (read >= 0) {
try {
if (fKilled) {
diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/StreamsProxy.java b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/StreamsProxy.java
index 616e796..19df55e 100644
--- a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/StreamsProxy.java
+++ b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/StreamsProxy.java
@@ -15,6 +15,7 @@
import java.io.IOException;
+import java.nio.charset.Charset;
import org.eclipse.debug.core.model.IStreamMonitor;
import org.eclipse.debug.core.model.IStreamsProxy;
@@ -43,20 +44,21 @@
* <code>false</code> by default.
*/
private boolean fClosed;
+
/**
- * Creates a <code>StreamsProxy</code> on the streams
- * of the given system process.
+ * Creates a <code>StreamsProxy</code> on the streams of the given system
+ * process.
*
* @param process system process to create a streams proxy on
- * @param encoding the process's encoding or <code>null</code> if default
+ * @param charset the process's charset or <code>null</code> if default
*/
- public StreamsProxy(Process process, String encoding) {
+ public StreamsProxy(Process process, Charset charset) {
if (process == null) {
return;
}
- fOutputMonitor= new OutputStreamMonitor(process.getInputStream(), encoding);
- fErrorMonitor= new OutputStreamMonitor(process.getErrorStream(), encoding);
- fInputMonitor= new InputStreamMonitor(process.getOutputStream(), encoding);
+ fOutputMonitor = new OutputStreamMonitor(process.getInputStream(), charset);
+ fErrorMonitor = new OutputStreamMonitor(process.getErrorStream(), charset);
+ fInputMonitor = new InputStreamMonitor(process.getOutputStream(), charset);
fOutputMonitor.startMonitoring();
fErrorMonitor.startMonitoring();
fInputMonitor.startMonitoring();
diff --git a/org.eclipse.debug.tests/META-INF/MANIFEST.MF b/org.eclipse.debug.tests/META-INF/MANIFEST.MF
index 3b3c6bf..665c778 100644
--- a/org.eclipse.debug.tests/META-INF/MANIFEST.MF
+++ b/org.eclipse.debug.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.debug.tests;singleton:=true
-Bundle-Version: 3.11.500.qualifier
+Bundle-Version: 3.11.600.qualifier
Bundle-Activator: org.eclipse.debug.tests.TestsPlugin
Bundle-Localization: plugin
Require-Bundle: org.eclipse.ui;bundle-version="[3.6.0,4.0.0)",
diff --git a/org.eclipse.debug.tests/pom.xml b/org.eclipse.debug.tests/pom.xml
index 7d2b577..d45cc86 100644
--- a/org.eclipse.debug.tests/pom.xml
+++ b/org.eclipse.debug.tests/pom.xml
@@ -18,7 +18,7 @@
</parent>
<groupId>org.eclipse.debug</groupId>
<artifactId>org.eclipse.debug.tests</artifactId>
- <version>3.11.500-SNAPSHOT</version>
+ <version>3.11.600-SNAPSHOT</version>
<packaging>eclipse-test-plugin</packaging>
<properties>
<code.ignoredWarnings>${tests.ignoredWarnings}</code.ignoredWarnings>
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/StreamsProxyTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/StreamsProxyTests.java
index b4ffdb4..6f0b2af 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/StreamsProxyTests.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/StreamsProxyTests.java
@@ -69,7 +69,7 @@
final String s = prefix + String.join("", Collections.nCopies(numTwoByteCharacters, "\u00F8"));
final ByteArrayInputStream stdout = new ByteArrayInputStream(s.getBytes(StandardCharsets.UTF_8));
final Process mockProcess = new MockProcess(stdout, null, 0);
- final StreamsProxy streamProxy = new StreamsProxy(mockProcess, StandardCharsets.UTF_8.name());
+ final StreamsProxy streamProxy = new StreamsProxy(mockProcess, StandardCharsets.UTF_8);
streamProxy.close();
final String readContent = streamProxy.getOutputStreamMonitor().getContents();
assertEquals("Process output got corrupted.", s, readContent);
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java
index 9c7b3c7..6034b3e 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java
@@ -630,7 +630,11 @@
}
/**
- * This class listens to a specified IO stream
+ * This class listens to a specified stream monitor to get notified on output
+ * from the process connected to console.
+ * <p>
+ * Received output will be redirected to given {@link IOConsoleOutputStream} to
+ * get it shown in console and to {@link #fFileOutputStream} if set.
*/
private class StreamListener implements IStreamListener {
@@ -640,9 +644,8 @@
private String fStreamId;
- private boolean fFlushed = false;
-
- private boolean fListenerRemoved = false;
+ /** Flag to remember if stream was already closed. */
+ private boolean fStreamClosed = false;
public StreamListener(String streamIdentifier, IStreamMonitor monitor, IOConsoleOutputStream stream) {
this.fStreamId = streamIdentifier;
@@ -650,58 +653,49 @@
this.fStream = stream;
fStreamMonitor.addListener(this);
//fix to bug 121454. Ensure that output to fast processes is processed.
- streamAppended(null, monitor);
+ flushAndDisableBuffer(monitor);
+ }
+
+ /**
+ * Process existing content in monitor and flush and disable buffering if it is
+ * a {@link IFlushableStreamMonitor}.
+ *
+ * @param monitor the monitor which might have buffered content
+ */
+ private void flushAndDisableBuffer(IStreamMonitor monitor) {
+ String contents;
+ synchronized (monitor) {
+ contents = monitor.getContents();
+ if (monitor instanceof IFlushableStreamMonitor) {
+ IFlushableStreamMonitor m = (IFlushableStreamMonitor) monitor;
+ m.flushContents();
+ m.setBuffered(false);
+ }
+ }
+ streamAppended(contents, monitor);
}
@Override
public void streamAppended(String text, IStreamMonitor monitor) {
- String encoding = getEncoding();
- if (fFlushed) {
- try {
- if (fStream != null) {
- if (encoding == null) {
- fStream.write(text);
+ if (text == null || text.length() == 0) {
+ return;
+ }
+ try {
+ if (fStream != null) {
+ fStream.write(text);
+ }
+ if (fFileOutputStream != null) {
+ Charset charset = getCharset();
+ synchronized (fFileOutputStream) {
+ if (charset == null) {
+ fFileOutputStream.write(text.getBytes());
} else {
- fStream.write(text.getBytes(encoding));
+ fFileOutputStream.write(text.getBytes(charset));
}
}
- if (fFileOutputStream != null) {
- synchronized (fFileOutputStream) {
- if (encoding == null) {
- fFileOutputStream.write(text.getBytes());
- } else {
- fFileOutputStream.write(text.getBytes(encoding));
- }
- }
- }
- } catch (IOException e) {
- DebugUIPlugin.log(e);
- }
- } else {
- String contents = null;
- synchronized (fStreamMonitor) {
- fFlushed = true;
- contents = fStreamMonitor.getContents();
- if (fStreamMonitor instanceof IFlushableStreamMonitor) {
- IFlushableStreamMonitor m = (IFlushableStreamMonitor) fStreamMonitor;
- m.flushContents();
- m.setBuffered(false);
- }
}
- try {
- if (contents != null && contents.length() > 0) {
- if (fStream != null) {
- fStream.write(contents);
- }
- if (fFileOutputStream != null) {
- synchronized (fFileOutputStream) {
- fFileOutputStream.write(contents.getBytes());
- }
- }
- }
- } catch (IOException e) {
- DebugUIPlugin.log(e);
- }
+ } catch (IOException e) {
+ DebugUIPlugin.log(e);
}
}
@@ -711,22 +705,20 @@
}
synchronized (fStreamMonitor) {
fStreamMonitor.removeListener(this);
- if (!fFlushed) {
- String contents = fStreamMonitor.getContents();
- streamAppended(contents, fStreamMonitor);
- }
- fListenerRemoved = true;
+ fStreamClosed = true;
+
try {
if (fStream != null) {
fStream.close();
}
} catch (IOException e) {
+ DebugUIPlugin.log(e);
}
}
}
public void dispose() {
- if (!fListenerRemoved) {
+ if (!fStreamClosed) {
closeStream();
}
fStream = null;
