Bug 558489 - [console] Remove unnecessary recoding in ProcessConsoleI20200102-1800I20200101-1800I20191231-1800I20191230-1800I20191229-2200I20191229-1800I20191228-1800I20191227-1800I20191226-1800I20191225-1800I20191224-1800I20191223-1800I20191222-1800I20191221-2205I20191221-1800

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>

8 files changed, 90 insertions, 82 deletions

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 e731732cb..8ee0a099d 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 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 			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 199b183d8..94e2fb733 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 @@ package org.eclipse.debug.internal.core;
 
 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 @@ public class InputStreamMonitor {
 	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 @@ public class InputStreamMonitor {
 	}
 
 	/**
-	 * 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 @@ public class InputStreamMonitor {
 			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 34aa133a8..00daec44d 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.BufferedInputStream;
 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 @@ public class OutputStreamMonitor implements IFlushableStreamMonitor {
 
 	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 @@ public class OutputStreamMonitor implements IFlushableStreamMonitor {
 		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 616e79674..19df55e28 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 @@ package org.eclipse.debug.internal.core;
 
 
 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 @@ public class StreamsProxy implements IStreamsProxy, IStreamsProxy2 {
 	 * <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 3b3c6bff9..665c778ee 100644
--- a/org.eclipse.debug.tests/META-INF/MANIFEST.MF
+++ b/org.eclipse.debug.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 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 7d2b5779a..d45cc86db 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 b4ffdb4a4..6f0b2af78 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 @@ public class StreamsProxyTests extends AbstractDebugTest {
 		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 9c7b3c7d6..6034b3efa 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 @@ public class ProcessConsole extends IOConsole implements IConsole, IDebugEventSe
 	}
 
 	/**
-	 * 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 @@ public class ProcessConsole extends IOConsole implements IConsole, IDebugEventSe
 
 		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 @@ public class ProcessConsole extends IOConsole implements IConsole, IDebugEventSe
 			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);
-						} else {
-							fStream.write(text.getBytes(encoding));
-						}
-					}
-					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);
-					}
+			if (text == null || text.length() == 0) {
+				return;
+			}
+			try {
+				if (fStream != null) {
+					fStream.write(text);
 				}
-				try {
-					if (contents != null && contents.length() > 0) {
-						if (fStream != null) {
-							fStream.write(contents);
-						}
-						if (fFileOutputStream != null) {
-							synchronized (fFileOutputStream) {
-								fFileOutputStream.write(contents.getBytes());
-							}
+				if (fFileOutputStream != null) {
+					Charset charset = getCharset();
+					synchronized (fFileOutputStream) {
+						if (charset == null) {
+							fFileOutputStream.write(text.getBytes());
+						} else {
+							fFileOutputStream.write(text.getBytes(charset));
 						}
 					}
-				} catch (IOException e) {
-					DebugUIPlugin.log(e);
 				}
+			} catch (IOException e) {
+				DebugUIPlugin.log(e);
 			}
 		}
 
@@ -711,22 +705,20 @@ public class ProcessConsole extends IOConsole implements IConsole, IDebugEventSe
 			}
 			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;