From c8c41dd67aee5185a97978afbe306f4a4eb0ec17 Mon Sep 17 00:00:00 2001 From: Erwin Waterlander Date: Fri, 23 Jun 2023 15:21:49 +0100 Subject: [PATCH] Postpone build container creation to build start. Fixes possible Eclipse freeze. 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 https://github.com/eclipse-cdt/cdt/issues/424 Fixes issue #424 --- .../org.eclipse.cdt.core/META-INF/MANIFEST.MF | 2 +- .../cdt/core/build/CBuildConfiguration.java | 50 +++++++++++++---- .../build/StandardBuildConfiguration.java | 54 ++++++++++--------- 3 files changed, 72 insertions(+), 34 deletions(-) diff --git a/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF b/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF index 8fe27c74ce9..82693b61112 100644 --- a/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF +++ b/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 diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java index 9f38926a0ee..0b0c397ee9d 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java +++ b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java @@ -232,20 +232,49 @@ public List 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 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); diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java index 05a3e82d07a..da5726bfef1 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java +++ b/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; } /**