Do not throw on launch cancel #2009

Fixes #2009.

`ILaunchConfiguration.launch()` javadoc declares that ILaunch is
returned even when `IProgressMonitor` argument is canceled.

Before this fix, `OperationCanceledException` could be thrown instead.

Author: basilevs

debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchConfiguration.java

@@ -45,6 +45,7 @@
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.OperationCanceledException;
 import org.eclipse.core.runtime.PlatformObject;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.SubMonitor;
@@ -775,7 +776,17 @@ public ILaunch launch(String mode, IProgressMonitor monitor, boolean build, bool
 				/* Launch the delegate */
 				lmonitor.subTask(DebugCoreMessages.LaunchConfiguration_2);
 				delegate.launch(this, mode, launch, lmonitor.split(10));
+			} catch (OperationCanceledException e) {
+				if (launch != null) {
+					getLaunchManager().removeLaunch(launch);
+					return launch;
+				}
+				throw e;
 			} catch (CoreException e) {
+				if (launch != null && e.getStatus().matches(IStatus.CANCEL)) {
+					getLaunchManager().removeLaunch(launch);
+					return launch;
+				}
 				// if there was an exception, and the launch is empty, remove it
 				if (launch != null && !launch.hasChildren()) {
 					getLaunchManager().removeLaunch(launch);

debug/org.eclipse.debug.tests/src/org/eclipse/debug/tests/launching/LaunchConfigurationTests.java

@@ -17,6 +17,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
@@ -42,6 +43,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.eclipse.core.filesystem.EFS;
 import org.eclipse.core.filesystem.IFileSystem;
@@ -55,7 +57,10 @@
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.core.runtime.Platform;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences;
 import org.eclipse.core.runtime.preferences.InstanceScope;
 import org.eclipse.debug.core.DebugPlugin;
@@ -64,9 +69,11 @@
 import org.eclipse.debug.core.ILaunchConfigurationListener;
 import org.eclipse.debug.core.ILaunchConfigurationType;
 import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
+import org.eclipse.debug.core.ILaunchDelegate;
 import org.eclipse.debug.core.ILaunchManager;
 import org.eclipse.debug.core.ILaunchesListener2;
 import org.eclipse.debug.core.Launch;
+import org.eclipse.debug.core.model.ILaunchConfigurationDelegate2;
 import org.eclipse.debug.core.model.IProcess;
 import org.eclipse.debug.internal.core.LaunchConfiguration;
 import org.eclipse.debug.internal.core.LaunchManager;
@@ -1331,6 +1338,79 @@ public void launchesTerminated(ILaunch[] launches) {
 		}
 	}
 
+	/**
+	 * Do not return null or throw on cancel
+	 *
+	 * @see https://github.com/eclipse-platform/eclipse.platform/issues/2009
+	 * @see org.eclipse.debug.core.ILaunchConfiguration#launch(String,
+	 *      IProgressMonitor)
+	 */
+	@Test
+	public void testCancel() throws CoreException {
+		final ILaunchDelegate launchDelegate = ((LaunchManager) DebugPlugin.getDefault().getLaunchManager()).getLaunchDelegate(LaunchConfigurationTests.ID_TEST_LAUNCH_TYPE);
+		final TestLaunchDelegate testLaunchDelegate = (TestLaunchDelegate) launchDelegate.getDelegate();
+		ILaunchConfigurationDelegate2 customLaunchDelegate = new ILaunchConfigurationDelegate2() {
+
+			void sleep() {
+			}
+			@Override
+			public void launch(ILaunchConfiguration configuration, String mode, ILaunch launch, IProgressMonitor monitor) throws CoreException {
+				sleep();
+				if (monitor.isCanceled()) {
+					throw new CoreException(Status.CANCEL_STATUS);
+				}
+			}
+
+			@Override
+			public boolean preLaunchCheck(ILaunchConfiguration configuration, String mode, IProgressMonitor monitor) throws CoreException {
+				sleep();
+				return !monitor.isCanceled();
+			}
+
+			@Override
+			public ILaunch getLaunch(ILaunchConfiguration configuration, String mode) throws CoreException {
+				sleep();
+				return null;
+			}
+
+			@Override
+			public boolean finalLaunchCheck(ILaunchConfiguration configuration, String mode, IProgressMonitor monitor) throws CoreException {
+				sleep();
+				return !monitor.isCanceled();
+			}
+
+			@Override
+			public boolean buildForLaunch(ILaunchConfiguration configuration, String mode, IProgressMonitor monitor) throws CoreException {
+				sleep();
+				return !monitor.isCanceled();
+			}
+		};
+		testLaunchDelegate.setDelegate(customLaunchDelegate);
+		try {
+			ILaunchConfigurationWorkingCopy workingCopy = newConfiguration(null, "cancel me"); //$NON-NLS-1$
+			int cancelCount = 0;
+			for (int i = 0; i < 10_000; i++) {
+				AtomicInteger checkCount = new AtomicInteger(i);
+				NullProgressMonitor monitor = new NullProgressMonitor() {
+					@Override
+					public boolean isCanceled() {
+						return super.isCanceled() || checkCount.getAndDecrement() <= 0;
+					}
+				};
+				// Should not throw
+				ILaunch launch = workingCopy.launch(ILaunchManager.RUN_MODE, monitor);
+				assertNotNull(launch);
+				if (!monitor.isCanceled()) {
+					break;
+				}
+				cancelCount++;
+			}
+			assertNotEquals(0, cancelCount);
+		} finally {
+			testLaunchDelegate.setDelegate(null);
+		}
+	}
+
 	/**
 	 * Tests that a launch is properly registered for notifications before a
 	 * process is spawned and may already propagate a termination event.