Bug 542701 - NPE at LaunchConfiguration.getLaunchManager

- add new logic to ContainerTypeProvider to use a JobGroup with
  maximum 1 thread so jobs from this class are never run
  at the same time
- change ContainerTypeProvider.getStatus() to wait until there are no
  scheduled, active, or cancelled jobs in the JobGroup
- change ContainerLaunchConfigurationDelegate.finalLaunchCheck() to
  create a fake launch target and call LaunchTargetManager.getStatus()
  which will eventually call the new ContainerTypeProvider.getStatus()
- fix NLS warnings in ContainerLaunchConfigurationDelegate

Change-Id: Icad11c5814dd8ee7fac99a41d74329ce65907a38

5 files changed, 84 insertions, 21 deletions

diff --git a/launch/org.eclipse.cdt.docker.launcher/META-INF/MANIFEST.MF b/launch/org.eclipse.cdt.docker.launcher/META-INF/MANIFEST.MF
index eaebc7645b9..c6ad6b21759 100644
--- a/launch/org.eclipse.cdt.docker.launcher/META-INF/MANIFEST.MF
+++ b/launch/org.eclipse.cdt.docker.launcher/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Plugin.name
 Bundle-SymbolicName: org.eclipse.cdt.docker.launcher;singleton:=true
-Bundle-Version: 1.2.500.qualifier
+Bundle-Version: 1.2.600.qualifier
 Bundle-Activator: org.eclipse.cdt.docker.launcher.DockerLaunchUIPlugin
 Bundle-Vendor: %Plugin.vendor
 Bundle-Localization: plugin
diff --git a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/docker/launcher/ContainerTargetTypeProvider.java b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/docker/launcher/ContainerTargetTypeProvider.java
index 4491501dd61..371e62d9b4a 100644
--- a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/docker/launcher/ContainerTargetTypeProvider.java
+++ b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/docker/launcher/ContainerTargetTypeProvider.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2017, 2019 QNX Software Systems and others.
+ * Copyright (c) 2017, 2020 QNX Software Systems and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -27,6 +27,7 @@ import org.eclipse.cdt.core.build.IToolChain;
 import org.eclipse.cdt.core.build.IToolChainManager;
 import org.eclipse.cdt.core.build.IToolChainProvider;
 import org.eclipse.cdt.debug.core.CDebugCorePlugin;
+import org.eclipse.cdt.internal.docker.launcher.Messages;
 import org.eclipse.cdt.internal.docker.launcher.ui.launchbar.ContainerGCCToolChain;
 import org.eclipse.cdt.internal.docker.launcher.ui.launchbar.ContainerGCCToolChainProvider;
 import org.eclipse.cdt.make.core.MakeCorePlugin;
@@ -36,6 +37,7 @@ import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.core.runtime.jobs.JobGroup;
 import org.eclipse.launchbar.core.ILaunchBarManager;
 import org.eclipse.launchbar.core.target.ILaunchTarget;
 import org.eclipse.launchbar.core.target.ILaunchTargetManager;
@@ -59,6 +61,9 @@ public class ContainerTargetTypeProvider implements ILaunchTargetProvider, IDock
 	public static final String TYPE_ID = "org.eclipse.cdt.docker.launcher.launchTargetType.container"; //$NON-NLS-1$
 	public static final String CONTAINER_LINUX = "linux-container"; //$NON-NLS-1$
 
+	private static final JobGroup containerTargetTypeGroup = new JobGroup(
+			Messages.ContainerTargetTypeProviderJobGroup_name, 1, 1);
+
 	private ILaunchTargetManager targetManager;
 
 	@Override
@@ -139,6 +144,7 @@ public class ContainerTargetTypeProvider implements ILaunchTargetProvider, IDock
 				return Status.OK_STATUS;
 			}
 		};
+		checkConfigs.setJobGroup(containerTargetTypeGroup);
 		checkConfigs.setUser(true);
 		checkConfigs.schedule();
 
@@ -153,6 +159,16 @@ public class ContainerTargetTypeProvider implements ILaunchTargetProvider, IDock
 
 	@Override
 	public TargetStatus getStatus(ILaunchTarget target) {
+		// don't allow status to be ok until all check jobs are complete
+		// we could do a join() here but to be absolutely sure we don't cause a deadlock, we will sleep
+		// until there are no jobs scheduled
+		while (containerTargetTypeGroup.getState() != JobGroup.NONE) {
+			try {
+				Thread.sleep(200);
+			} catch (InterruptedException e) {
+				// do nothing
+			}
+		}
 		// Always OK
 		return TargetStatus.OK_STATUS;
 	}
@@ -268,6 +284,7 @@ public class ContainerTargetTypeProvider implements ILaunchTargetProvider, IDock
 			}
 		};
 		checkConfigs.setUser(true);
+		checkConfigs.setJobGroup(containerTargetTypeGroup);
 		checkConfigs.schedule();
 	}
 
diff --git a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchConfigurationDelegate.java b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchConfigurationDelegate.java
index c8cc207b615..ae370e8f98d 100644
--- a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchConfigurationDelegate.java
+++ b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchConfigurationDelegate.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2015, 2016 Red Hat and others.
+ * Copyright (c) 2015, 2020 Red Hat and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -58,6 +58,7 @@ import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
 import org.eclipse.debug.core.ILaunchManager;
 import org.eclipse.launchbar.core.target.ILaunchTarget;
 import org.eclipse.launchbar.core.target.ILaunchTargetManager;
+import org.eclipse.launchbar.core.target.ILaunchTargetWorkingCopy;
 import org.eclipse.linuxtools.docker.core.Activator;
 import org.eclipse.linuxtools.docker.core.IDockerContainerInfo;
 import org.eclipse.linuxtools.docker.core.IDockerNetworkSettings;
@@ -69,6 +70,8 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 
 	private ContainerLauncher launcher;
 
+	private final static String NONE = ""; //$NON-NLS-1$
+
 	private class StartGdbServerJob extends Job implements IContainerLaunchListener {
 
 		private boolean started;
@@ -207,7 +210,7 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 						IPath path = new Path(additionalDir);
 						String dir = path.toPortableString();
 						if (path.getDevice() != null) {
-							dir = "/" + dir.replace(':', '/');
+							dir = "/" + dir.replace(':', '/'); //$NON-NLS-1$
 						}
 						dirs.add(dir);
 					}
@@ -243,7 +246,7 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 				}
 
 				String image = configuration.getAttribute(ILaunchConstants.ATTR_IMAGE, (String) null);
-				String connectionUri = configuration.getAttribute(ILaunchConstants.ATTR_CONNECTION_URI, "");
+				String connectionUri = configuration.getAttribute(ILaunchConstants.ATTR_CONNECTION_URI, NONE);
 				boolean keepContainer = configuration.getAttribute(ILaunchConstants.ATTR_KEEP_AFTER_LAUNCH, false);
 
 				boolean supportStdin = configuration.getAttribute(ILaunchConstants.ATTR_STDIN_SUPPORT, false);
@@ -308,7 +311,7 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 
 				StringBuilder b = new StringBuilder();
 
-				b.append(gdbserverCommand).append(' ').append(commandArguments); //$NON-NLS-1$
+				b.append(gdbserverCommand).append(' ').append(commandArguments);
 
 				String arguments = getProgramArguments(configuration);
 				if (arguments.trim().length() > 0) {
@@ -348,7 +351,7 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 						IPath path = new Path(additionalDir);
 						String dir = path.toPortableString();
 						if (path.getDevice() != null) {
-							dir = "/" + dir.replace(':', '/');
+							dir = "/" + dir.replace(':', '/'); //$NON-NLS-1$
 						}
 						dirs.add(dir);
 					}
@@ -356,10 +359,10 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 				}
 
 				String image = configuration.getAttribute(ILaunchConstants.ATTR_IMAGE, (String) null);
-				String connectionUri = configuration.getAttribute(ILaunchConstants.ATTR_CONNECTION_URI, "");
+				String connectionUri = configuration.getAttribute(ILaunchConstants.ATTR_CONNECTION_URI, NONE);
 				boolean isLocalConnection = true;
 				try {
-					Pattern ipaddrPattern = Pattern.compile("[a-z]*://([0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+)[:]*[0-9]*");
+					Pattern ipaddrPattern = Pattern.compile("[a-z]*://([0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+)[:]*[0-9]*"); //$NON-NLS-1$
 					Matcher m = ipaddrPattern.matcher(connectionUri);
 					if (m.matches()) {
 						String ipaddr = m.group(1);
@@ -448,7 +451,7 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 	 * @throws CoreException
 	 */
 	private String getProgramArguments(ILaunchConfiguration config) throws CoreException {
-		String args = config.getAttribute(ICDTLaunchConfigurationConstants.ATTR_PROGRAM_ARGUMENTS, "");
+		String args = config.getAttribute(ICDTLaunchConfigurationConstants.ATTR_PROGRAM_ARGUMENTS, NONE);
 		if (args != null && args.length() > 0) {
 			args = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(args);
 		}
@@ -463,13 +466,13 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 	 * @throws CoreException
 	 */
 	private IPath getCommandPath(ILaunchConfiguration configuration) throws CoreException {
-		String projectName = configuration.getAttribute(ICDTLaunchConfigurationConstants.ATTR_PROJECT_NAME, "");
+		String projectName = configuration.getAttribute(ICDTLaunchConfigurationConstants.ATTR_PROJECT_NAME, NONE);
 		if (projectName.length() > 0) {
 			IProject project = CCorePlugin.getWorkspace().getRoot().getProject(projectName);
 			if (project == null)
 				return null;
 
-			String name = configuration.getAttribute(ICDTLaunchConfigurationConstants.ATTR_PROGRAM_NAME, "");
+			String name = configuration.getAttribute(ICDTLaunchConfigurationConstants.ATTR_PROGRAM_NAME, NONE);
 
 			if (name.length() == 0)
 				return null;
@@ -574,7 +577,7 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 			ILaunchTarget target = null;
 			ILaunchTarget[] targets = targetManager.getLaunchTargetsOfType(ContainerTargetTypeProvider.TYPE_ID);
 			for (ILaunchTarget t : targets) {
-				if (t.getAttribute(IContainerLaunchTarget.ATTR_IMAGE_ID, "").replaceAll(":", "_").equals(m.group(1))) {
+				if (t.getAttribute(IContainerLaunchTarget.ATTR_IMAGE_ID, "").replaceAll(":", "_").equals(m.group(1))) { //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 					target = t;
 					break;
 				}
@@ -601,11 +604,48 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 		IProject project = getProject(configuration);
 		ILaunchTargetManager targetManager = CCorePlugin.getService(ILaunchTargetManager.class);
 		ILaunchTarget target = null;
+		// force ContainerTargetTypeProvider to flush all check jobs by calling getStatus()
+		ILaunchTarget fakeTarget = new ILaunchTarget() {
+
+			@Override
+			public <T> T getAdapter(Class<T> adapter) {
+				return null;
+			}
+
+			@Override
+			public String getId() {
+				return "org.eclipse.cdt.docker.launch.fakeTarget"; //$NON-NLS-1$
+			}
+
+			@Override
+			public String getTypeId() {
+				return ContainerTargetTypeProvider.TYPE_ID;
+			}
+
+			@Override
+			public String getAttribute(String key, String defValue) {
+				return defValue;
+			}
+
+			@Override
+			public Map<String, String> getAttributes() {
+				return Collections.emptyMap();
+			}
+
+			@Override
+			public ILaunchTargetWorkingCopy getWorkingCopy() {
+				throw new UnsupportedOperationException();
+			}
+
+		};
+		targetManager.getStatus(fakeTarget);
+
+		// now that check jobs are flushed, get launch targets
 		ILaunchTarget[] targets = targetManager.getLaunchTargetsOfType(ContainerTargetTypeProvider.TYPE_ID);
 		String image = configuration.getAttribute(IContainerLaunchTarget.ATTR_IMAGE_ID, (String) null);
 		String connection = configuration.getAttribute(IContainerLaunchTarget.ATTR_CONNECTION_URI, (String) null);
 		for (ILaunchTarget t : targets) {
-			if (t.getAttribute(IContainerLaunchTarget.ATTR_IMAGE_ID, "").equals(image)) {
+			if (t.getAttribute(IContainerLaunchTarget.ATTR_IMAGE_ID, "").equals(image)) { //$NON-NLS-1$
 				target = t;
 				break;
 			}
@@ -642,12 +682,14 @@ public class ContainerLaunchConfigurationDelegate extends GdbLaunchDelegate {
 		IProject project = null;
 		if (projectName == null) {
 			IResource[] resources = config.getMappedResources();
-			if (resources != null && resources.length > 0 && resources[0] instanceof IProject) {
-				project = (IProject) resources[0];
+			if (resources != null && resources.length > 0) {
+				project = resources[0].getProject();
+			}
+			if (project != null) {
+				ILaunchConfigurationWorkingCopy wc = config.getWorkingCopy();
+				wc.setAttribute(ICDTLaunchConfigurationConstants.ATTR_PROJECT_NAME, project.getName());
+				wc.doSave();
 			}
-			ILaunchConfigurationWorkingCopy wc = config.getWorkingCopy();
-			wc.setAttribute(ICDTLaunchConfigurationConstants.ATTR_PROJECT_NAME, project.getName());
-			wc.doSave();
 		} else {
 			projectName = projectName.trim();
 			if (!projectName.isEmpty()) {
diff --git a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/Messages.java b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/Messages.java
index 3d2565222d2..62070e41dca 100644
--- a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/Messages.java
+++ b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/Messages.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2018 Red Hat, Inc.
+ * Copyright (c) 2012, 2020 Red Hat, Inc.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -59,6 +59,8 @@ public class Messages extends NLS {
 	public static String ContainerTab_Warning_Connection_Not_Found;
 	public static String ContainerTab_Warning_Image_Not_Found;
 
+	public static String ContainerTargetTypeProviderJobGroup_name;
+
 	public static String HeaderPreferencePage_Connection_Label;
 	public static String HeaderPreferencePage_Image_Label;
 	public static String HeaderPreferencePage_Remove_Label;
diff --git a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/messages.properties b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/messages.properties
index 4a9999226d8..4743982b1b2 100644
--- a/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/messages.properties
+++ b/launch/org.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/messages.properties
@@ -1,5 +1,5 @@
 #*******************************************************************************
-# Copyright (c) 2015, 2018 Red Hat.
+# Copyright (c) 2015, 2020 Red Hat.
 #
 # This program and the accompanying materials
 # are made available under the terms of the Eclipse Public License 2.0
@@ -52,6 +52,8 @@ ContainerTab_Error_No_Images=No Docker Images exist
 ContainerTab_Warning_Connection_Not_Found=Docker Connection: {0} for Launch Configuration not found: defaulting to {1}
 ContainerTab_Warning_Image_Not_Found=Docker Image: {0} is not a valid pulled image in current Connection: {1}
 
+ContainerTargetTypeProviderJobGroup_name=Detecting and checking for valid Container Launch Targets
+
 ContainerPortDialog_shellTitle=Exposing a Container Port
 ContainerPortDialog_explanationLabel=Specify the container port to expose:
 ContainerPortDialog_containerLabel=Container port: