Bug 578302: Remove empty nodes from launch managerY20220121-0600

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>

5 files changed, 241 insertions, 56 deletions

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 3bc997eb0..bc9cdfea3 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 @@ public class LaunchConfiguration extends PlatformObject implements ILaunchConfig
 			}
 			// 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;
+			try {
+				if (delegate2 != null) {
+					launch = delegate2.getLaunch(this, mode);
 				}
-			}
-			lmonitor.setWorkRemaining(22);
-		// perform pre-launch build
-			if (build) {
-				lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_7 + DebugCoreMessages.LaunchConfiguration_6);
-				boolean tempbuild = build;
+				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) {
-					tempbuild = delegate2.buildForLaunch(this, mode, lmonitor.split(7));
+					if (!(delegate2.preLaunchCheck(this, mode, lmonitor.split(1)))) {
+						getLaunchManager().removeLaunch(launch);
+						return launch;
+					}
 				}
-				if (tempbuild) {
-					lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_7 + DebugCoreMessages.LaunchConfiguration_5);
-					ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, lmonitor.split(3));
+				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(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);
+				lmonitor.setWorkRemaining(11);
 
-			try {
 				//initialize the source locator
 				lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_3);
 				initializeSourceLocator(launch);
@@ -803,13 +803,13 @@ public class LaunchConfiguration extends PlatformObject implements ILaunchConfig
 				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 8fcb3b9f3..56ca2e3b1 100755
--- a/org.eclipse.debug.tests/plugin.properties
+++ b/org.eclipse.debug.tests/plugin.properties
@@ -21,4 +21,5 @@ renderingType.name.1 = Rendering Default
 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 4503162f5..3738652a0 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 3f6160db8..0b2bc0045 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 @@ package org.eclipse.debug.tests.launching;
 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.ILaunchConfigurationWorkingCopy;
 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,16 +258,17 @@ public class LaunchManagerTests extends AbstractLaunchTest {
 	}
 
 	/**
-	 * Checks if the expected number of cancelled launches appear in the manager
+	 * Checks if the expected number of <i>type</i> launches appear in the
+	 * manager
 	 *
 	 * @param count the expected count
-	 * @since 3.9.100
+	 * @param type the type of launches to count
 	 */
-	void hasCancellingLaunches(int count) {
+	private void hasLaunches(Class<?> type, int count) {
 		ILaunch[] launches = getLaunchManager().getLaunches();
 		int num = 0;
 		for (ILaunch launche : launches) {
-			if (launche instanceof CancellingLaunch) {
+			if (type.isInstance(launche)) {
 				num++;
 			}
 		}
@@ -272,6 +276,16 @@ public class LaunchManagerTests extends AbstractLaunchTest {
 	}
 
 	/**
+	 * 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) {
+		hasLaunches(CancellingLaunch.class, count);
+	}
+
+	/**
 	 * Tests if a launch is properly removed from the launch manager when
 	 * #preLaunchCheck is cancelled
 	 *
@@ -400,4 +414,96 @@ public class LaunchManagerTests extends AbstractLaunchTest {
 			}
 		}
 	}
+
+	/**
+	 * 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 000000000..98cfd14c1
--- /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;
+	}
+}