Postpone build container creation to build start. Fixes possible Ecli…

…pse freeze. (#433)

The creation of the build container for Core Build projects is
postponed to the start of the build process.

StandardBuildConfiguration getBuildContainer and setBuildContainer
have been cleaned up.

CBuildConfiguration creation is started via
CBuildConfigurationManager.getBuildConfiguration(IBuildConfiguration)
which holds a lock on the HashMap 'configs'. Creation of
StandardBuildConfiguration triggered, via applyProperties and
getBuildContainer(), a Folder.create which loops back to
CBuildConfigurationManager.getBuildConfiguration().
For detailed traces see #424

Fixes #424

core/org.eclipse.cdt.core/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.cdt.core; singleton:=true
-Bundle-Version: 8.2.100.qualifier
+Bundle-Version: 8.2.200.qualifier
 Bundle-Activator: org.eclipse.cdt.core.CCorePlugin
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java

@@ -232,20 +232,49 @@ public List<String> getBinaryParserIds() throws CoreException {
 		return toolChain != null ? toolChain.getBinaryParserIds() : List.of(CCorePlugin.DEFAULT_BINARY_PARSER_UNIQ_ID);
 	}
 
+	/**
+	 * Create build container if it doesn't exist yet.
+	 *
+	 * For a {@link StandardBuildConfiguration} the container can be the project or
+	 * a folder. For all other Core Build configurations it is a folder.
+	 *
+	 * @param container Container where build is executed.
+	 * @param monitor
+	 * @throws CoreException
+	 */
+	/*
+	 * Container creation is done at the start of the build process. Folder creation
+	 * during the creation of a Core Build Configuration can lead to a loop back to
+	 * CBuildConfigurationManager.getBuildConfiguration() that makes Eclipse freeze.
+	 * This was seen with StandardBuildConfiguration when it created the build
+	 * container. Folder.create got triggered via applyProperties() and
+	 * getBuildContainer(). See https://github.com/eclipse-cdt/cdt/issues/424
+	 */
+	private void createBuildContainer(IContainer container, IProgressMonitor monitor) throws CoreException {
+		if (!(container instanceof IProject) && !container.exists()) {
+			IContainer parent = container.getParent();
+			if (!(parent instanceof IProject) && !parent.exists()) {
+				createBuildContainer(parent, monitor);
+			}
+
+			if (container instanceof IFolder) {
+				((IFolder) container).create(IResource.FORCE | IResource.DERIVED, true, monitor);
+			}
+		}
+	}
+
+	/*
+	 * Only the StandardBuildConfiguration has an option in the launch configuration
+	 * to change the build container to the project root. For as long as the
+	 * StandardBuildConfiguration is the only one there is no need to make the
+	 * folder name a project property for all types. StandardBuildConfiguration has
+	 * its own getBuildContainer() override.
+	 */
 	public IContainer getBuildContainer() throws CoreException {
-		// TODO make the name of this folder a project property
 		IProject project = getProject();
 		IFolder buildRootFolder = project.getFolder("build"); //$NON-NLS-1$
 		IFolder buildFolder = buildRootFolder.getFolder(name);
 
-		IProgressMonitor monitor = new NullProgressMonitor();
-		if (!buildRootFolder.exists()) {
-			buildRootFolder.create(IResource.FORCE | IResource.DERIVED, true, monitor);
-		}
-		if (!buildFolder.exists()) {
-			buildFolder.create(IResource.FORCE | IResource.DERIVED, true, monitor);
-		}
-
 		return buildFolder;
 	}
 
@@ -521,6 +550,9 @@ public Process startBuildProcess(List<String> commands, IEnvironmentVariable[] e
 			IConsole console, IProgressMonitor monitor) throws IOException, CoreException {
 		Process process = null;
 		IToolChain tc = getToolChain();
+
+		createBuildContainer(getBuildContainer(), monitor);
+
 		if (tc instanceof IToolChain2) {
 			process = ((IToolChain2) tc).startBuildProcess(this, commands, buildDirectory.toString(), envVars, console,
 					monitor);

core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java

@@ -84,16 +84,20 @@ public StandardBuildConfiguration(IBuildConfiguration config, String name, ITool
 		setupEnvVars();
 	}
 
-	private void applyProperties() {
-		String container = getProperty(BUILD_CONTAINER);
-		if (container != null && !container.trim().isEmpty()) {
-			IPath containerLoc = new org.eclipse.core.runtime.Path(container);
+	private IContainer getContainer(String containerPath) {
+		if (containerPath != null && !containerPath.trim().isEmpty()) {
+			IPath containerLoc = new org.eclipse.core.runtime.Path(containerPath);
 			if (containerLoc.segmentCount() == 1) {
-				buildContainer = ResourcesPlugin.getWorkspace().getRoot().getProject(containerLoc.segment(0));
+				return ResourcesPlugin.getWorkspace().getRoot().getProject(containerLoc.segment(0));
 			} else {
-				buildContainer = ResourcesPlugin.getWorkspace().getRoot().getFolder(containerLoc);
+				return ResourcesPlugin.getWorkspace().getRoot().getFolder(containerLoc);
 			}
 		}
+		return null;
+	}
+
+	private void applyProperties() {
+		setBuildContainer(getProperty(BUILD_CONTAINER));
 
 		String buildCmd = getProperty(BUILD_COMMAND);
 		if (buildCmd != null && !buildCmd.trim().isEmpty()) {
@@ -137,9 +141,26 @@ public IEnvironmentVariable[] getVariables() {
 		return envVars;
 	}
 
+	/**
+	 * Set the build container based on the full path starting from workspace root.
+	 *
+	 * @param containerPath Path from workspace root.
+	 */
+	private void setBuildContainer(String containerPath) {
+		IContainer container = null;
+		if (containerPath != null && !containerPath.trim().isEmpty()) {
+			container = getContainer(containerPath);
+		}
+		setBuildContainer(container);
+	}
+
 	public void setBuildContainer(IContainer buildContainer) {
 		this.buildContainer = buildContainer;
-		setProperty(BUILD_CONTAINER, buildContainer.getFullPath().toString());
+		if (buildContainer == null) {
+			setProperty(BUILD_CONTAINER, ""); // overwrite old property value.
+		} else {
+			setProperty(BUILD_CONTAINER, buildContainer.getFullPath().toString());
+		}
 	}
 
 	public void setBuildCommand(String[] buildCommand) {
@@ -162,27 +183,12 @@ public void setCleanCommand(String[] cleanCommand) {
 		}
 	}
 
-	private void createBuildContainer(IContainer container, IProgressMonitor monitor) throws CoreException {
-		IContainer parent = container.getParent();
-		if (!(parent instanceof IProject) && !parent.exists()) {
-			createBuildContainer(parent, monitor);
-		}
-
-		if (container instanceof IFolder) {
-			((IFolder) container).create(IResource.FORCE | IResource.DERIVED, true, monitor);
-		}
-	}
-
 	@Override
 	public IContainer getBuildContainer() throws CoreException {
 		if (buildContainer == null) {
-			return super.getBuildContainer();
-		} else {
-			if (!(buildContainer instanceof IProject) && !buildContainer.exists()) {
-				createBuildContainer(buildContainer, new NullProgressMonitor());
-			}
+			setBuildContainer(getDefaultBuildContainer());
 		}
-		return buildContainer != null ? buildContainer : super.getBuildContainer();
+		return buildContainer;
 	}
 
 	/**