Bug 578302: Remove empty nodes from launch manager If the delegate thorws an exception, remove the launch from the launch manager if it does not contain any children. This should ensure there is not launch nodes left in the debug view that cannot be removed. Contributed by STMicroelectronics Change-Id: I0543135a495f673e351f86ebbd11efcbe96caff5 Signed-off-by: Torbjörn Svensson <torbjorn.svensson@st.com> Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/189838 Tested-by: Platform Bot <platform-bot@eclipse.org> Reviewed-by: Sarika Sinha <sarika.sinha@in.ibm.com>
diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchConfiguration.java b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchConfiguration.java index 3bc997e..bc9cdfe 100644 --- a/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchConfiguration.java +++ b/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchConfiguration.java
@@ -737,62 +737,62 @@ } // allow the delegate to provide a launch implementation ILaunch launch = null; - if (delegate2 != null) { - launch = delegate2.getLaunch(this, mode); - } - if (launch == null) { - launch = new Launch(this, mode, null); - } else // ensure the launch mode is valid - if (!mode.equals(launch.getLaunchMode())) { - IStatus status = new Status(IStatus.ERROR, DebugPlugin.getUniqueIdentifier(), DebugPlugin.ERROR, - MessageFormat.format(DebugCoreMessages.LaunchConfiguration_14, mode, launch.getLaunchMode()), null); - throw new CoreException(status); - } - launch.setAttribute(DebugPlugin.ATTR_LAUNCH_TIMESTAMP, Long.toString(System.currentTimeMillis())); - boolean captureOutput = getAttribute(DebugPlugin.ATTR_CAPTURE_OUTPUT, true); - if(!captureOutput) { - launch.setAttribute(DebugPlugin.ATTR_CAPTURE_OUTPUT, "false"); //$NON-NLS-1$ - } else { - launch.setAttribute(DebugPlugin.ATTR_CAPTURE_OUTPUT, null); - } - launch.setAttribute(DebugPlugin.ATTR_CONSOLE_ENCODING, getLaunchManager().getEncoding(this)); - if (register) { - getLaunchManager().addLaunch(launch); - } - // perform initial pre-launch sanity checks - lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_8); - - if (delegate2 != null) { - if (!(delegate2.preLaunchCheck(this, mode, lmonitor.split(1)))) { - getLaunchManager().removeLaunch(launch); - return launch; - } - } - lmonitor.setWorkRemaining(22); - // perform pre-launch build - if (build) { - lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_7 + DebugCoreMessages.LaunchConfiguration_6); - boolean tempbuild = build; - if (delegate2 != null) { - tempbuild = delegate2.buildForLaunch(this, mode, lmonitor.split(7)); - } - if (tempbuild) { - lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_7 + DebugCoreMessages.LaunchConfiguration_5); - ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, lmonitor.split(3)); - } - } - lmonitor.setWorkRemaining(12); - // final validation - lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_4); - if (delegate2 != null) { - if (!(delegate2.finalLaunchCheck(this, mode, lmonitor.split(1)))) { - getLaunchManager().removeLaunch(launch); - return launch; - } - } - lmonitor.setWorkRemaining(11); - try { + if (delegate2 != null) { + launch = delegate2.getLaunch(this, mode); + } + if (launch == null) { + launch = new Launch(this, mode, null); + } else // ensure the launch mode is valid + if (!mode.equals(launch.getLaunchMode())) { + IStatus status = new Status(IStatus.ERROR, DebugPlugin.getUniqueIdentifier(), DebugPlugin.ERROR, + MessageFormat.format(DebugCoreMessages.LaunchConfiguration_14, mode, launch.getLaunchMode()), null); + throw new CoreException(status); + } + launch.setAttribute(DebugPlugin.ATTR_LAUNCH_TIMESTAMP, Long.toString(System.currentTimeMillis())); + boolean captureOutput = getAttribute(DebugPlugin.ATTR_CAPTURE_OUTPUT, true); + if (!captureOutput) { + launch.setAttribute(DebugPlugin.ATTR_CAPTURE_OUTPUT, "false"); //$NON-NLS-1$ + } else { + launch.setAttribute(DebugPlugin.ATTR_CAPTURE_OUTPUT, null); + } + launch.setAttribute(DebugPlugin.ATTR_CONSOLE_ENCODING, getLaunchManager().getEncoding(this)); + if (register) { + getLaunchManager().addLaunch(launch); + } + // perform initial pre-launch sanity checks + lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_8); + + if (delegate2 != null) { + if (!(delegate2.preLaunchCheck(this, mode, lmonitor.split(1)))) { + getLaunchManager().removeLaunch(launch); + return launch; + } + } + lmonitor.setWorkRemaining(22); + // perform pre-launch build + if (build) { + lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_7 + DebugCoreMessages.LaunchConfiguration_6); + boolean tempbuild = build; + if (delegate2 != null) { + tempbuild = delegate2.buildForLaunch(this, mode, lmonitor.split(7)); + } + if (tempbuild) { + lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_7 + DebugCoreMessages.LaunchConfiguration_5); + ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, lmonitor.split(3)); + } + } + lmonitor.setWorkRemaining(12); + // final validation + lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_4); + if (delegate2 != null) { + if (!(delegate2.finalLaunchCheck(this, mode, lmonitor.split(1)))) { + getLaunchManager().removeLaunch(launch); + return launch; + } + } + lmonitor.setWorkRemaining(11); + //initialize the source locator lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_3); initializeSourceLocator(launch); @@ -803,13 +803,13 @@ delegate.launch(this, mode, launch, lmonitor.split(10)); } catch (CoreException e) { // if there was an exception, and the launch is empty, remove it - if (!launch.hasChildren()) { + if (launch != null && !launch.hasChildren()) { getLaunchManager().removeLaunch(launch); } throw e; } catch (RuntimeException e) { // if there was a runtime exception, and the launch is empty, remove it - if (!launch.hasChildren()) { + if (launch != null && !launch.hasChildren()) { getLaunchManager().removeLaunch(launch); } throw e;
diff --git a/org.eclipse.debug.tests/plugin.properties b/org.eclipse.debug.tests/plugin.properties index 8fcb3b9..56ca2e3 100755 --- a/org.eclipse.debug.tests/plugin.properties +++ b/org.eclipse.debug.tests/plugin.properties
@@ -21,4 +21,5 @@ launchConfigurationType.name = Test Launch Type extension.name = Debug File System launchConfigurationType.name.0 = Cancelling Launch Type +launchConfigurationType.name.1 = Throwing Launch Type testBreakpoint.name = Test Breakpoint \ No newline at end of file
diff --git a/org.eclipse.debug.tests/plugin.xml b/org.eclipse.debug.tests/plugin.xml index 4503162..3738652 100644 --- a/org.eclipse.debug.tests/plugin.xml +++ b/org.eclipse.debug.tests/plugin.xml
@@ -71,6 +71,12 @@ modes="run" name="%launchConfigurationType.name.0"> </launchConfigurationType> + <launchConfigurationType + delegate="org.eclipse.debug.tests.launching.ThrowingLaunchDelegate" + id="throwing.type" + modes="run" + name="%launchConfigurationType.name.1"> + </launchConfigurationType> </extension> <extension id="debugFS"
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java index 3f6160d..0b2bc00 100644 --- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java +++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchManagerTests.java
@@ -16,6 +16,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -29,6 +30,8 @@ import org.eclipse.debug.core.ILaunchManager; import org.eclipse.debug.internal.core.LaunchManager; import org.eclipse.debug.tests.launching.CancellingLaunchDelegate.CancellingLaunch; +import org.eclipse.debug.tests.launching.ThrowingLaunchDelegate.ThrowingEnum; +import org.eclipse.debug.tests.launching.ThrowingLaunchDelegate.ThrowingLaunch; import org.junit.Test; /** @@ -255,20 +258,31 @@ } /** + * Checks if the expected number of <i>type</i> launches appear in the + * manager + * + * @param count the expected count + * @param type the type of launches to count + */ + private void hasLaunches(Class<?> type, int count) { + ILaunch[] launches = getLaunchManager().getLaunches(); + int num = 0; + for (ILaunch launche : launches) { + if (type.isInstance(launche)) { + num++; + } + } + assertEquals("The number of expected launches is wrong", count, num); //$NON-NLS-1$ + } + + /** * Checks if the expected number of cancelled launches appear in the manager * * @param count the expected count * @since 3.9.100 */ void hasCancellingLaunches(int count) { - ILaunch[] launches = getLaunchManager().getLaunches(); - int num = 0; - for (ILaunch launche : launches) { - if (launche instanceof CancellingLaunch) { - num++; - } - } - assertEquals("The number of expected launches is wrong", count, num); //$NON-NLS-1$ + hasLaunches(CancellingLaunch.class, count); } /** @@ -400,4 +414,96 @@ } } } + + /** + * Create a new configuration that will throw exception in one of the four + * launch delegate methods + * + * @param throwingEnum the method that should throw exception + * @return the new {@link ILaunchConfiguration} + */ + private ILaunchConfiguration getThrowingConfiguration(ThrowingEnum throwingEnum) throws Exception { + ILaunchConfigurationType type = getLaunchManager().getLaunchConfigurationType("throwing.type"); //$NON-NLS-1$ + if (type != null) { + ILaunchConfigurationWorkingCopy copy = type.newInstance(null, getLaunchManager().generateLaunchConfigurationName("throwing " + throwingEnum)); //$NON-NLS-1$ + copy.setAttribute("throw.preLaunchCheck", ThrowingEnum.preLaunchCheck.equals(throwingEnum)); //$NON-NLS-1$ + copy.setAttribute("throw.finalLaunchCheck", ThrowingEnum.finalLaunchCheck.equals(throwingEnum)); //$NON-NLS-1$ + copy.setAttribute("throw.buildForLaunch", ThrowingEnum.buildForLaunch.equals(throwingEnum)); //$NON-NLS-1$ + copy.setAttribute("throw.launch", ThrowingEnum.launch.equals(throwingEnum)); //$NON-NLS-1$ + return copy.doSave(); + } + return null; + } + + /** + * Tests if a launch is properly removed from the launch manager when + * <i>throwingEnum</i> method throws exception + * + * @throws Exception + * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=578302 + */ + private void testThrowingLaunchDelegateMethod(ThrowingEnum throwingEnum) throws Exception { + ILaunchConfiguration config = getThrowingConfiguration(throwingEnum); + assertNotNull("The throwing config should have been created", config); //$NON-NLS-1$ + try { + hasLaunches(ThrowingLaunch.class, 0); + CoreException exception = assertThrows(CoreException.class, () -> config.launch("run", new NullProgressMonitor(), true, true)); //$NON-NLS-1$ + assertEquals("Wrong method throwed exception", exception.getMessage(), throwingEnum.toString()); + hasLaunches(ThrowingLaunch.class, 0); + } finally { + ILaunch[] launches = getLaunchManager().getLaunches(); + for (ILaunch launche : launches) { + getLaunchManager().removeLaunch(launche); + } + config.delete(); + } + } + + /** + * Tests if a launch is properly removed from the launch manager when + * #preLaunchCheck throws exception + * + * @throws Exception + * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=578302 + */ + @Test + public void testThrowingPreLaunchCheck() throws Exception { + testThrowingLaunchDelegateMethod(ThrowingEnum.preLaunchCheck); + } + + /** + * Tests if a launch is properly removed from the launch manager when + * #finalLaunchCheck throws exception + * + * @throws Exception + * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=578302 + */ + @Test + public void testThrowingFinalLaunchCheck() throws Exception { + testThrowingLaunchDelegateMethod(ThrowingEnum.finalLaunchCheck); + } + + /** + * Tests if a launch is properly removed from the launch manager when + * #buildFoLaunch throws exception + * + * @throws Exception + * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=578302 + */ + @Test + public void testThrowingBuildForLaunch() throws Exception { + testThrowingLaunchDelegateMethod(ThrowingEnum.buildForLaunch); + } + + /** + * Tests if a launch is properly removed from the launch manager when + * #buildFoLaunch throws exception + * + * @throws Exception + * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=578302 + */ + @Test + public void testThrowingLaunch() throws Exception { + testThrowingLaunchDelegateMethod(ThrowingEnum.launch); + } }
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/ThrowingLaunchDelegate.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/ThrowingLaunchDelegate.java new file mode 100644 index 0000000..98cfd14 --- /dev/null +++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/ThrowingLaunchDelegate.java
@@ -0,0 +1,72 @@ +/******************************************************************************* + * Copyright (c) 2014 IBM Corporation 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: + * IBM Corporation - initial API and implementation + *******************************************************************************/ +package org.eclipse.debug.tests.launching; + +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.Status; +import org.eclipse.debug.core.ILaunch; +import org.eclipse.debug.core.ILaunchConfiguration; +import org.eclipse.debug.core.Launch; +import org.eclipse.debug.core.model.ILaunchConfigurationDelegate2; +import org.eclipse.debug.core.model.ISourceLocator; + +public class ThrowingLaunchDelegate implements ILaunchConfigurationDelegate2 { + + class ThrowingLaunch extends Launch { + public ThrowingLaunch(ILaunchConfiguration launchConfiguration, String mode, ISourceLocator locator) { + super(launchConfiguration, mode, locator); + } + } + + enum ThrowingEnum { + launch, buildForLaunch, finalLaunchCheck, preLaunchCheck; + } + + @Override + public void launch(ILaunchConfiguration configuration, String mode, ILaunch launch, IProgressMonitor monitor) throws CoreException { + if (configuration.getAttribute("throw.launch", true)) { //$NON-NLS-1$ + throw new CoreException(Status.error(ThrowingEnum.launch.toString())); + } + } + + @Override + public ILaunch getLaunch(ILaunchConfiguration configuration, String mode) throws CoreException { + return new ThrowingLaunch(configuration, "run", null); //$NON-NLS-1$ + } + + @Override + public boolean buildForLaunch(ILaunchConfiguration configuration, String mode, IProgressMonitor monitor) throws CoreException { + if (configuration.getAttribute("throw.buildForLaunch", true)) { //$NON-NLS-1$ + throw new CoreException(Status.error(ThrowingEnum.buildForLaunch.toString())); + } + return true; + } + + @Override + public boolean finalLaunchCheck(ILaunchConfiguration configuration, String mode, IProgressMonitor monitor) throws CoreException { + if (configuration.getAttribute("throw.finalLaunchCheck", true)) { //$NON-NLS-1$ + throw new CoreException(Status.error(ThrowingEnum.finalLaunchCheck.toString())); + } + return true; + } + + @Override + public boolean preLaunchCheck(ILaunchConfiguration configuration, String mode, IProgressMonitor monitor) throws CoreException { + if (configuration.getAttribute("throw.preLaunchCheck", true)) { //$NON-NLS-1$ + throw new CoreException(Status.error(ThrowingEnum.preLaunchCheck.toString())); + } + return true; + } +}