diff options
author | Paul Pazderski | 2019-04-28 14:01:27 +0000 |
---|---|---|
committer | Andrey Loskutov | 2019-04-28 16:52:42 +0000 |
commit | f09f1e9f05795fb820e3c859273faae755ce94e2 (patch) | |
tree | fe50093a3a9c8c5f0e939fed6067f99c30d6af7f | |
parent | 72f672fe7e6becae36c083ab3f9162f47476b128 (diff) | |
download | eclipse.platform.debug-I20190501-0525.tar.gz eclipse.platform.debug-I20190501-0525.tar.xz eclipse.platform.debug-I20190501-0525.zip |
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; } |