From ee99fbeaf29e81f32dbd96402362b7072d7467cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20L=C3=A4ubrich?= <laeubi@laeubi-soft.de>
Date: Mon, 10 Mar 2025 07:20:59 +0100
Subject: [PATCH] Only start the stream monitoring after events are send out

Currently it is almost impossible to capture the full binary stream of a
process even if one adds a listener right after creation of the stream
proxy. The reason is that the streams are immediately started to being
read in the constructor. The code currently uses some caching in place
but this only works reliable for a single listener and is then reset
afterwards having other missing data depending on how fast they can
register. Even worse there is a small chance that after registration and
before reading the buffer some bytes are received and then is processed
before the buffer contents.

This now creates a new constructor that only creates the stream but do
not start them. Instead users must call the method startMonitoring for
this. The RuntimeProcess now uses this constructor to delay the start of
stream reading unless all notifications are performed so other parties
can register accordingly.
---
 .../debug/core/model/RuntimeProcess.java      | 18 +++++-
 .../debug/internal/core/StreamsProxy.java     | 55 +++++++++++++++----
 2 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/debug/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java b/debug/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java
index 9e457accd3d..aa9d830cb69 100644
--- a/debug/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java
+++ b/debug/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java
@@ -166,6 +166,22 @@ public RuntimeProcess(ILaunch launch, Process process, String name, Map<String,
 		launch.addProcess(this);
 		fMonitor.start();
 		fireCreationEvent();
+		startStreams();
+	}
+
+	/**
+	 * Called after notification that this runtime process is created to start
+	 * the reading of streams. Must be overridden if a custom
+	 * {@link IStreamsProxy} instance is provided by
+	 * {@link #createStreamsProxy()} that do not extends {@link StreamsProxy}
+	 * and does not start the stream reading in any other mean.
+	 *
+	 * @since 3.23
+	 */
+	protected void startStreams() {
+		if (fStreamsProxy instanceof StreamsProxy impl) {
+			impl.startMonitoring(fThreadNameSuffix);
+		}
 	}
 
 	private static String getPidInfo(Process process, ILaunch launch) {
@@ -389,7 +405,7 @@ protected IStreamsProxy createStreamsProxy() {
 		if (charset == null) {
 			charset = Platform.getSystemCharset();
 		}
-		return new StreamsProxy(getSystemProcess(), charset, fThreadNameSuffix);
+		return new StreamsProxy(getSystemProcess(), charset);
 	}
 
 	/**
diff --git a/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/StreamsProxy.java b/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/StreamsProxy.java
index 4c636238aaf..0806eb53d18 100644
--- a/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/StreamsProxy.java
+++ b/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/StreamsProxy.java
@@ -15,6 +15,8 @@
 
 
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.nio.charset.Charset;
 
 import org.eclipse.debug.core.model.IBinaryStreamMonitor;
@@ -33,43 +35,75 @@ public class StreamsProxy implements IBinaryStreamsProxy {
 	/**
 	 * The monitor for the output stream (connected to standard out of the process)
 	 */
-	private OutputStreamMonitor fOutputMonitor;
+	private final OutputStreamMonitor fOutputMonitor;
 	/**
 	 * The monitor for the error stream (connected to standard error of the process)
 	 */
-	private OutputStreamMonitor fErrorMonitor;
+	private final OutputStreamMonitor fErrorMonitor;
 	/**
 	 * The monitor for the input stream (connected to standard in of the process)
 	 */
-	private InputStreamMonitor fInputMonitor;
+	private final InputStreamMonitor fInputMonitor;
 	/**
 	 * Records the open/closed state of communications with
 	 * the underlying streams.  Note: fClosed is initialized to
 	 * <code>false</code> by default.
 	 */
 	private boolean fClosed;
+	private boolean started;
 
 	/**
 	 * Creates a <code>StreamsProxy</code> on the streams of the given system
-	 * process.
+	 * process and starts the monitoring.
 	 *
 	 * @param process system process to create a streams proxy on
 	 * @param charset the process's charset or <code>null</code> if default
 	 * @param suffix Thread name suffix
 	 */
-	@SuppressWarnings("resource")
 	public StreamsProxy(Process process, Charset charset, String suffix) {
+		this(process, charset);
+		startMonitoring(suffix);
+	}
+
+	/**
+	 * Creates a <code>StreamsProxy</code> on the streams of the given system
+	 * process, monitoring must be started separately.
+	 *
+	 * @param process system process to create a streams proxy on
+	 * @param charset the process's charset or <code>null</code> if default
+	 */
+	@SuppressWarnings("resource")
+	public StreamsProxy(Process process, Charset charset) {
 		if (process == null) {
-			return;
+			fOutputMonitor = new OutputStreamMonitor(InputStream.nullInputStream(), charset);
+			fErrorMonitor = new OutputStreamMonitor(InputStream.nullInputStream(), charset);
+			fInputMonitor = new InputStreamMonitor(OutputStream.nullOutputStream(), charset);
+		} else {
+			fOutputMonitor = new OutputStreamMonitor(process.getInputStream(), charset);
+			fErrorMonitor = new OutputStreamMonitor(process.getErrorStream(), charset);
+			fInputMonitor = new InputStreamMonitor(process.getOutputStream(), charset);
 		}
-		fOutputMonitor = new OutputStreamMonitor(process.getInputStream(), charset);
-		fErrorMonitor = new OutputStreamMonitor(process.getErrorStream(), charset);
-		fInputMonitor = new InputStreamMonitor(process.getOutputStream(), charset);
+	}
+
+	/**
+	 * Starts the monitoring of streams using the given suffix
+	 *
+	 * @param suffix
+	 */
+	public void startMonitoring(String suffix) {
+		start();
 		fOutputMonitor.startMonitoring("Output Stream Monitor" + suffix); //$NON-NLS-1$
 		fErrorMonitor.startMonitoring("Error Stream Monitor" + suffix); //$NON-NLS-1$
 		fInputMonitor.startMonitoring("Input Stream Monitor" + suffix); //$NON-NLS-1$
 	}
 
+	private synchronized void start() {
+		if (started) {
+			throw new IllegalStateException("Already started!"); //$NON-NLS-1$
+		}
+		started = true;
+	}
+
 	/**
 	 * Creates a <code>StreamsProxy</code> on the streams of the given system
 	 * process.
@@ -78,7 +112,7 @@ public StreamsProxy(Process process, Charset charset, String suffix) {
 	 * @param encoding the process's encoding or <code>null</code> if default
 	 * @deprecated use {@link #StreamsProxy(Process, Charset, String)} instead
 	 */
-	@Deprecated
+	@Deprecated(forRemoval = true, since = "2025-06")
 	public StreamsProxy(Process process, String encoding) {
 		// This constructor was once removed in favor of the Charset variant
 		// but Bug 562653 brought up a client which use this internal class via
@@ -178,4 +212,5 @@ public void write(byte[] data, int offset, int length) throws IOException {
 			throw new IOException();
 		}
 	}
+
 }