Bug 546710 - [console] Race condition on process console creation
Change-Id: I473f4fa1296cbbc823f6d2b6ad6827957816d64c
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>
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 7d87356..4b1ddce 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.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 @@
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 a07a505..7bd305e 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.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 @@
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 0000000..93eb5b3
--- /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 4468db1..3ac3fa9 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.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 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 fb73e6e..45d3ac7 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 @@
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 @@
// 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;
}
