Bug 546710 - [console] Race condition on process console creationI20190501-1800I20190501-0525I20190430-1800I20190429-1800I20190429-0240I20190428-1800

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

5 files changed, 176 insertions, 6 deletions

diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/AutomatedSuite.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/AutomatedSuite.java
index 7d873562c..4b1ddcee8 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/AutomatedSuite.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/AutomatedSuite.java
@@ -19,6 +19,7 @@ import org.eclipse.debug.tests.breakpoint.BreakpointOrderingTests;
 import org.eclipse.debug.tests.console.ConsoleDocumentAdapterTests;
 import org.eclipse.debug.tests.console.ConsoleManagerTests;
 import org.eclipse.debug.tests.console.ConsoleTests;
+import org.eclipse.debug.tests.console.ProcessConsoleManagerTests;
 import org.eclipse.debug.tests.console.ProcessConsoleTests;
 import org.eclipse.debug.tests.launching.AcceleratorSubstitutionTests;
 import org.eclipse.debug.tests.launching.ArgumentParsingTests;
@@ -113,6 +114,7 @@ public class AutomatedSuite extends TestSuite {
 		addTest(new TestSuite(ConsoleManagerTests.class));
 		addTest(new TestSuite(ConsoleTests.class));
 		addTest(new TestSuite(ProcessConsoleTests.class));
+		addTest(new TestSuite(ProcessConsoleManagerTests.class));
 
 		// Launch Groups
 		addTest(new TestSuite(LaunchGroupTests.class));
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcess.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcess.java
index a07a5059c..7bd305e18 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcess.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcess.java
@@ -19,6 +19,11 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.eclipse.debug.core.DebugPlugin;
+import org.eclipse.debug.core.ILaunchManager;
+import org.eclipse.debug.core.Launch;
+import org.eclipse.debug.core.model.RuntimeProcess;
+
 /**
  * A mockup process which can either simulate generation of output or wait for
  * input to read.
@@ -194,4 +199,31 @@ public class MockProcess extends Process {
 	private boolean isTerminated() {
 		return endTime != RUN_FOREVER && System.currentTimeMillis() > endTime;
 	}
+
+	/**
+	 * Create a {@link RuntimeProcess} which wraps this {@link MockProcess}.
+	 * <p>
+	 * Note: the process will only be connected to a minimal dummy launch
+	 * object.
+	 * </p>
+	 *
+	 * @return the created {@link RuntimeProcess}
+	 */
+	public RuntimeProcess toRuntimeProcess() {
+		return toRuntimeProcess("MockProcess");
+	}
+
+	/**
+	 * Create a {@link RuntimeProcess} which wraps this {@link MockProcess}.
+	 * <p>
+	 * Note: the process will only be connected to a minimal dummy launch
+	 * object.
+	 * </p>
+	 *
+	 * @param name a custom name for the process
+	 * @return the created {@link RuntimeProcess}
+	 */
+	public RuntimeProcess toRuntimeProcess(String name) {
+		return (RuntimeProcess) DebugPlugin.newProcess(new Launch(null, ILaunchManager.RUN_MODE, null), this, name);
+	}
 }
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleManagerTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleManagerTests.java
new file mode 100644
index 000000000..93eb5b362
--- /dev/null
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleManagerTests.java
@@ -0,0 +1,131 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Paul Pazderski and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Paul Pazderski - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.debug.tests.console;
+
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.debug.core.DebugPlugin;
+import org.eclipse.debug.core.ILaunch;
+import org.eclipse.debug.core.ILaunchListener;
+import org.eclipse.debug.core.ILaunchManager;
+import org.eclipse.debug.core.model.IProcess;
+import org.eclipse.debug.internal.core.LaunchManager;
+import org.eclipse.debug.internal.ui.DebugUIPlugin;
+import org.eclipse.debug.internal.ui.views.console.ConsoleRemoveAllTerminatedAction;
+import org.eclipse.debug.internal.ui.views.console.ProcessConsole;
+import org.eclipse.debug.internal.ui.views.console.ProcessConsoleManager;
+import org.eclipse.debug.tests.AbstractDebugTest;
+import org.eclipse.debug.tests.TestUtil;
+import org.eclipse.debug.ui.IDebugUIConstants;
+import org.eclipse.ui.console.ConsolePlugin;
+import org.eclipse.ui.console.IConsoleManager;
+
+/**
+ * Tests the ProcessConsoleManager.
+ */
+@SuppressWarnings("restriction")
+public class ProcessConsoleManagerTests extends AbstractDebugTest {
+
+	public ProcessConsoleManagerTests() {
+		super(ProcessConsoleManagerTests.class.getSimpleName());
+	}
+
+	public ProcessConsoleManagerTests(String name) {
+		super(name);
+	}
+
+	/**
+	 * Test addition and removal of a ProcessConsole. It also kind of tests
+	 * {@link LaunchManager} because the ProcessConsoleManager primary works
+	 * through an {@link ILaunchListener} which is honored by this test.
+	 */
+	public void testProcessConsoleLifecycle() throws Exception {
+		// ensure debug UI plugin is started before adding first launch
+		DebugUIPlugin.getDefault();
+		final ILaunchManager launchManager = DebugPlugin.getDefault().getLaunchManager();
+		final IConsoleManager consoleManager = ConsolePlugin.getDefault().getConsoleManager();
+		final int existingNumConsoles = consoleManager.getConsoles().length;
+		if (existingNumConsoles > 0) {
+			// existing consoles must not harm this test but it may be
+			// interesting in case the test fails
+			TestUtil.log(IStatus.INFO, getName(), "Found " + existingNumConsoles + " existing consoles on test start.");
+		}
+
+		ILaunch launch = null;
+		final MockProcess mockProcess = new MockProcess(MockProcess.RUN_FOREVER);
+		try {
+			final IProcess process = mockProcess.toRuntimeProcess();
+			launch = process.getLaunch();
+			launchManager.addLaunch(launch);
+			// do not wait on input read job
+			TestUtil.waitForJobs(getName(), 0, 10000, ProcessConsole.class);
+			assertEquals("No console was added.", 1, consoleManager.getConsoles().length);
+		} finally {
+			mockProcess.destroy();
+		}
+
+		if (launch != null) {
+			launchManager.removeLaunch(launch);
+			TestUtil.waitForJobs(getName(), 0, 10000);
+			assertEquals("Console is not removed.", 0, consoleManager.getConsoles().length);
+		}
+	}
+
+	/**
+	 * Test problematic scenario where launch is already removed before console
+	 * is created. see https://bugs.eclipse.org/bugs/show_bug.cgi?id=546710#c13
+	 */
+	public void testBug546710_ConsoleCreationRaceCondition() throws Exception {
+		final ILaunchManager launchManager = DebugPlugin.getDefault().getLaunchManager();
+		for (ILaunch existingLaunch : launchManager.getLaunches()) {
+			assertTrue("Found existing not terminated launch. This should not happen and can interfere this test. Check for leakages in previous run tests.", existingLaunch.isTerminated());
+			launchManager.removeLaunch(existingLaunch);
+		}
+
+		final MockProcess mockProcess1 = new MockProcess(0);
+		final IProcess process1 = mockProcess1.toRuntimeProcess("FirstMockProcess");
+		final MockProcess mockProcess2 = new MockProcess(0);
+		final IProcess process2 = mockProcess2.toRuntimeProcess("SecondMockProcess");
+		final boolean prefRemoveOldLaunches = DebugUIPlugin.getDefault().getPreferenceStore().getBoolean(IDebugUIConstants.PREF_AUTO_REMOVE_OLD_LAUNCHES);
+		try {
+			DebugUIPlugin.getDefault().getPreferenceStore().setValue(IDebugUIConstants.PREF_AUTO_REMOVE_OLD_LAUNCHES, true);
+			// Stop the JobManager to reliable trigger the tested race
+			// condition.
+			Job.getJobManager().suspend();
+			launchManager.addLaunch(process1.getLaunch());
+			launchManager.addLaunch(process2.getLaunch());
+		} finally {
+			Job.getJobManager().resume();
+			DebugUIPlugin.getDefault().getPreferenceStore().setValue(IDebugUIConstants.PREF_AUTO_REMOVE_OLD_LAUNCHES, prefRemoveOldLaunches);
+		}
+
+		ProcessConsoleManager processConsoleManager = DebugUIPlugin.getDefault().getProcessConsoleManager();
+		TestUtil.waitForJobs(getName(), 0, 10000);
+		int openConsoles = 0;
+		if (processConsoleManager.getConsole(process1) != null) {
+			openConsoles++;
+		}
+		if (processConsoleManager.getConsole(process2) != null) {
+			openConsoles++;
+		}
+		assertEquals("ProcessConsoleManager and LaunchManager got out of sync.", openConsoles, launchManager.getLaunches().length);
+
+		final ConsoleRemoveAllTerminatedAction removeAction = new ConsoleRemoveAllTerminatedAction();
+		assertTrue("Remove terminated action should be enabled.", removeAction.isEnabled() || launchManager.getLaunches().length == 0);
+		removeAction.run();
+		TestUtil.waitForJobs(getName(), 0, 10000);
+		assertNull("First console not removed.", processConsoleManager.getConsole(process1));
+		assertNull("Second console not removed.", processConsoleManager.getConsole(process1));
+	}
+}
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 4468db1ac..3ac3fa989 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
@@ -19,9 +19,6 @@ import org.eclipse.core.runtime.ILogListener;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.jobs.Job;
-import org.eclipse.debug.core.DebugPlugin;
-import org.eclipse.debug.core.ILaunchManager;
-import org.eclipse.debug.core.Launch;
 import org.eclipse.debug.core.model.IProcess;
 import org.eclipse.debug.tests.AbstractDebugTest;
 import org.eclipse.debug.tests.TestUtil;
@@ -80,7 +77,7 @@ public class ProcessConsoleTests extends AbstractDebugTest {
 	public void testInputReadJobCancel() throws Exception {
 		final MockProcess mockProcess = new MockProcess(MockProcess.RUN_FOREVER);
 		try {
-			final IProcess process = DebugPlugin.newProcess(new Launch(null, ILaunchManager.RUN_MODE, null), mockProcess, "testInputReadJobCancel");
+			final IProcess process = mockProcess.toRuntimeProcess("testInputReadJobCancel");
 			@SuppressWarnings("restriction")
 			final org.eclipse.debug.internal.ui.views.console.ProcessConsole console = new org.eclipse.debug.internal.ui.views.console.ProcessConsole(process, new ConsoleColorProvider());
 			try {
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsoleManager.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsoleManager.java
index fb73e6e04..45d3ac769 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsoleManager.java
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsoleManager.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2015 IBM Corporation and others.
+ * Copyright (c) 2000, 2019 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -51,7 +51,7 @@ import com.ibm.icu.text.MessageFormat;
 public class ProcessConsoleManager implements ILaunchListener {
 
 	/**
-	 * Crates console for given process
+	 * Creates console for given process
 	 */
 	private final class ConsoleCreation extends Job {
 		private final ILaunch launch;
@@ -77,6 +77,14 @@ public class ProcessConsoleManager implements ILaunchListener {
 
 			// add new console to console manager.
 			ConsolePlugin.getDefault().getConsoleManager().addConsoles(new IConsole[] { pc });
+
+			// If a launch is removed the associated console is removed too. It can happen
+			// that the launch is removed even before the console could be created.
+			// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=546710#c13
+			ILaunchManager launchManager = DebugPlugin.getDefault().getLaunchManager();
+			if (!launchManager.isRegistered(launch)) {
+				removeLaunch(launch);
+			}
 			return Status.OK_STATUS;
 		}