Skip to content

Commit

Permalink
Bug 542701 - NPE at LaunchConfiguration.getLaunchManager
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@jjohnstn
jjohnstn committed May 26, 2020
1 parent 4fd1af6 commit a4f788a
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 21 deletions.
2 changes: 1 addition & 1 deletion launch/org.eclipse.cdt.docker.launcher/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.501.qualifier
Bundle-Activator: org.eclipse.cdt.docker.launcher.DockerLaunchUIPlugin
Bundle-Vendor: %Plugin.vendor
Bundle-Localization: plugin
Expand Down
19 changes: 18 additions & 1 deletion ....cdt.docker.launcher/src/org/eclipse/cdt/docker/launcher/ContainerTargetTypeProvider.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -27,6 +27,7 @@
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;
Expand All @@ -36,6 +37,7 @@
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;
Expand All @@ -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
Expand Down Expand Up @@ -139,6 +144,7 @@ protected IStatus run(IProgressMonitor monitor) {
return Status.OK_STATUS;
}
};
checkConfigs.setJobGroup(containerTargetTypeGroup);
checkConfigs.setUser(true);
checkConfigs.schedule();

Expand All @@ -153,6 +159,16 @@ protected IStatus run(IProgressMonitor monitor) {

@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;
}
Expand Down Expand Up @@ -268,6 +284,7 @@ protected IStatus run(IProgressMonitor monitor) {
}
};
checkConfigs.setUser(true);
checkConfigs.setJobGroup(containerTargetTypeGroup);
checkConfigs.schedule();
}

Expand Down
76 changes: 59 additions & 17 deletions ...er/src/org/eclipse/cdt/internal/docker/launcher/ContainerLaunchConfigurationDelegate.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -58,6 +58,7 @@
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;
Expand All @@ -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;
Expand Down Expand Up @@ -207,7 +210,7 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun
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);
}
Expand Down Expand Up @@ -243,7 +246,7 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun
}

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);
Expand Down Expand Up @@ -308,7 +311,7 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun

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) {
Expand Down Expand Up @@ -348,18 +351,18 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun
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);
}
additionalDirs = dirs;
}

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);
Expand Down Expand Up @@ -448,7 +451,7 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun
* @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);
}
Expand All @@ -463,13 +466,13 @@ private String getProgramArguments(ILaunchConfiguration config) throws CoreExcep
* @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;
Expand Down Expand Up @@ -574,7 +577,7 @@ public boolean buildForLaunch(ILaunchConfiguration configuration, String mode, I
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;
}
Expand All @@ -601,11 +604,48 @@ public boolean finalLaunchCheck(ILaunchConfiguration configuration, String mode,
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;
}
Expand Down Expand Up @@ -642,12 +682,14 @@ public boolean preLaunchCheck(ILaunchConfiguration config, String mode, IProgres
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()) {
Expand Down
4 changes: 3 additions & 1 deletion ...rg.eclipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/Messages.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion ...ipse.cdt.docker.launcher/src/org/eclipse/cdt/internal/docker/launcher/messages.properties
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit a4f788a

Please sign in to comment.