Bug 552015 - [console] Streams closed notification send to late if inputI20191013-1800

is connected to file

If process input is connected to file the 'all streams are closed'
notification was not send on process termination but later on console
removal.

Change-Id: I83a732e1c876d9c15b7274159c18dc7723ef5143
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>

4 files changed, 125 insertions, 5 deletions

diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTests.java
index c495892fa..0962231bd 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTests.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/IOConsoleTests.java
@@ -14,7 +14,9 @@
 package org.eclipse.debug.tests.console;
 
 import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -266,6 +268,22 @@ public class IOConsoleTests extends AbstractDebugTest {
 	}
 
 	/**
+	 * Test {@link IOConsole} with file as input source.
+	 */
+	public void testInputFile() throws Exception {
+		final IOConsoleTestUtil c = getTestUtil("Test input file");
+		// open default output stream to match usual behavior where two output
+		// streams are open and to prevent premature console closing
+		c.getDefaultOutputStream();
+		try (InputStream in = new ByteArrayInputStream(new byte[0])) {
+			c.getConsole().getInputStream().close();
+			c.getConsole().setInputStream(in);
+		}
+		closeConsole(c);
+		assertEquals("Test triggered errors in IOConsole.", 0, loggedErrors.get());
+	}
+
+	/**
 	 * Test mixes of outputs and user inputs in various variants.
 	 */
 	public void testMixedWriteAndInput() throws Exception {
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleTests.java
index ff66ca134..2227d43e7 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleTests.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleTests.java
@@ -13,9 +13,11 @@
  *******************************************************************************/
 package org.eclipse.debug.tests.console;
 
+import java.io.File;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.eclipse.core.runtime.ILogListener;
@@ -24,13 +26,24 @@ import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.debug.core.DebugPlugin;
 import org.eclipse.debug.core.ILaunch;
+import org.eclipse.debug.core.ILaunchConfiguration;
+import org.eclipse.debug.core.ILaunchConfigurationType;
+import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
 import org.eclipse.debug.core.ILaunchManager;
 import org.eclipse.debug.core.Launch;
 import org.eclipse.debug.core.model.IProcess;
+import org.eclipse.debug.internal.ui.DebugUIPlugin;
 import org.eclipse.debug.tests.AbstractDebugTest;
 import org.eclipse.debug.tests.TestUtil;
+import org.eclipse.debug.tests.launching.LaunchConfigurationTests;
+import org.eclipse.debug.ui.IDebugUIConstants;
 import org.eclipse.debug.ui.console.ConsoleColorProvider;
+import org.eclipse.jface.util.IPropertyChangeListener;
+import org.eclipse.jface.util.PropertyChangeEvent;
 import org.eclipse.ui.console.ConsolePlugin;
+import org.eclipse.ui.console.IConsole;
+import org.eclipse.ui.console.IConsoleConstants;
+import org.eclipse.ui.console.IConsoleManager;
 
 /**
  * Tests the ProcessConsole.
@@ -166,4 +179,72 @@ public class ProcessConsoleTests extends AbstractDebugTest {
 			mockProcess.destroy();
 		}
 	}
+
+	/**
+	 * Test console finished notification with standard process console.
+	 */
+	public void testProcessTerminationNotification() throws Exception {
+		TestUtil.log(IStatus.INFO, getName(), "Process terminates after Console is initialized.");
+		processTerminationTest(null, false);
+		TestUtil.log(IStatus.INFO, getName(), "Process terminates before Console is initialized.");
+		processTerminationTest(null, true);
+	}
+
+	/**
+	 * Test console finished notification if process standard input is feed from
+	 * file.
+	 */
+	public void testProcessTerminationNotificationWithInputFile() throws Exception {
+		File inFile = DebugUIPlugin.getDefault().getStateLocation().addTrailingSeparator().append("testStdin.txt").toFile();
+		boolean fileCreated = inFile.createNewFile();
+		assertTrue("Failed to prepare input file.", fileCreated);
+		try {
+			ILaunchConfigurationType launchType = DebugPlugin.getDefault().getLaunchManager().getLaunchConfigurationType(LaunchConfigurationTests.ID_TEST_LAUNCH_TYPE);
+			ILaunchConfigurationWorkingCopy launchConfiguration = launchType.newInstance(null, "testProcessTerminationNotificationWithInputFromFile");
+			launchConfiguration.setAttribute(IDebugUIConstants.ATTR_CAPTURE_STDIN_FILE, inFile.getAbsolutePath());
+			TestUtil.log(IStatus.INFO, getName(), "Process terminates after Console is initialized.");
+			processTerminationTest(launchConfiguration, false);
+			TestUtil.log(IStatus.INFO, getName(), "Process terminates before Console is initialized.");
+			processTerminationTest(launchConfiguration, true);
+		} finally {
+			inFile.delete();
+		}
+	}
+
+	/**
+	 * The shared code to test console finished notification.
+	 *
+	 * @param launchConfig <code>null</code> or configured with stdin file.
+	 * @param terminateBeforeConsoleInitialization if <code>true</code> the
+	 *            tested process is terminated before the ProcessConsole can
+	 *            perform its initialization. If <code>false</code> the process
+	 *            is guaranteed to run until the ProcessConsole was initialized.
+	 */
+	public void processTerminationTest(ILaunchConfiguration launchConfig, boolean terminateBeforeConsoleInitialization) throws Exception {
+		final AtomicBoolean terminationSignaled = new AtomicBoolean(false);
+		final Process mockProcess = new MockProcess(null, null, terminateBeforeConsoleInitialization ? 0 : -1);
+		final IProcess process = DebugPlugin.newProcess(new Launch(launchConfig, ILaunchManager.RUN_MODE, null), mockProcess, "testProcessTerminationNotification");
+		@SuppressWarnings("restriction")
+		final org.eclipse.debug.internal.ui.views.console.ProcessConsole console = new org.eclipse.debug.internal.ui.views.console.ProcessConsole(process, new ConsoleColorProvider());
+		console.addPropertyChangeListener(new IPropertyChangeListener() {
+			@Override
+			public void propertyChange(PropertyChangeEvent event) {
+				if (event.getSource() == console && IConsoleConstants.P_CONSOLE_OUTPUT_COMPLETE.equals(event.getProperty())) {
+					terminationSignaled.set(true);
+				}
+			}
+		});
+		final IConsoleManager consoleManager = ConsolePlugin.getDefault().getConsoleManager();
+		try {
+			consoleManager.addConsoles(new IConsole[] { console });
+			if (mockProcess.isAlive()) {
+				mockProcess.destroy();
+			}
+			TestUtil.waitForJobs(getName(), 50, 10000);
+			assertTrue("No console complete notification received.", terminationSignaled.get());
+		} finally {
+			consoleManager.removeConsoles(new IConsole[] { console });
+			TestUtil.waitForJobs(getName(), 0, 10000);
+		}
+	}
 }
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 4c8d38382..fa46e9bd9 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
@@ -11,6 +11,7 @@
  *  Contributors:
  *     IBM Corporation - initial API and implementation
  *     Paul Pazderski  - Bug 545769: fixed rare UTF-8 character corruption bug
+ *     Paul Pazderski  - Bug 552015: console finished signaled to late if input is connected to file
  *******************************************************************************/
 package org.eclipse.debug.internal.ui.views.console;
 
@@ -104,6 +105,16 @@ public class ProcessConsole extends IOConsole implements IConsole, IDebugEventSe
 
 	private IConsoleColorProvider fColorProvider;
 
+	/**
+	 * The input stream which can supply user input in console to the system process
+	 * stdin.
+	 */
+	private IOConsoleInputStream fUserInput;
+	/**
+	 * The stream connected to the system processe's stdin. May be the
+	 * <i>fUserInput</i> stream to supply user input or a FileInputStream to supply
+	 * input from a file.
+	 */
 	private volatile InputStream fInput;
 
 	private FileOutputStream fFileOutputStream;
@@ -114,14 +125,18 @@ public class ProcessConsole extends IOConsole implements IConsole, IDebugEventSe
 	private boolean fStreamsClosed = false;
 
 	/**
-	 * Proxy to a console document
+	 * Create process console with default encoding.
+	 *
+	 * @param process	   the process to associate with this console
+	 * @param colorProvider the colour provider for this console
 	 */
 	public ProcessConsole(IProcess process, IConsoleColorProvider colorProvider) {
 		this(process, colorProvider, null);
 	}
 
 	/**
-	 * Constructor
+	 * Create process console.
+	 *
 	 * @param process the process to associate with this console
 	 * @param colorProvider the colour provider for this console
 	 * @param encoding the desired encoding for this console
@@ -129,6 +144,7 @@ public class ProcessConsole extends IOConsole implements IConsole, IDebugEventSe
 	public ProcessConsole(IProcess process, IConsoleColorProvider colorProvider, String encoding) {
 		super(IInternalDebugCoreConstants.EMPTY_STRING, IDebugUIConstants.ID_PROCESS_CONSOLE_TYPE, null, encoding, true);
 		fProcess = process;
+		fUserInput = getInputStream();
 
 		ILaunchConfiguration configuration = process.getLaunch().getLaunchConfiguration();
 		String file = null;
@@ -421,6 +437,12 @@ public class ProcessConsole extends IOConsole implements IConsole, IDebugEventSe
 			fInput.close();
 		} catch (IOException e) {
 		}
+		if (fInput != fUserInput) {
+			try {
+				fUserInput.close();
+			} catch (IOException e) {
+			}
+		}
 		fStreamsClosed  = true;
 	}
 
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/ui/IDebugUIConstants.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/ui/IDebugUIConstants.java
index 1fd65d468..e30a6fc11 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/ui/IDebugUIConstants.java
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/ui/IDebugUIConstants.java
@@ -966,9 +966,8 @@ public interface IDebugUIConstants {
 	String ATTR_CAPTURE_IN_CONSOLE = PLUGIN_ID + ".ATTR_CONSOLE_OUTPUT_ON"; //$NON-NLS-1$
 
 	/**
-	 * Launch configuration boolean attribute specifying whether input for the
-	 * launched process will be captured from file. Default value is
-	 * <code>null</code>.
+	 * Launch configuration attribute to specifying a file whose content is supplied
+	 * to the launched process input stream. Default value is <code>null</code>.
 	 *
 	 * @since 3.11
 	 */