[259964] - race conditions with model creation - applying patch

3 files changed, 174 insertions, 68 deletions

diff --git a/build/org.eclipse.cdt.managedbuilder.core.tests/suite/org/eclipse/cdt/managedbuilder/tests/suite/AllManagedBuildTests.java b/build/org.eclipse.cdt.managedbuilder.core.tests/suite/org/eclipse/cdt/managedbuilder/tests/suite/AllManagedBuildTests.java
index 4f941a14a76..267a50613b5 100644
--- a/build/org.eclipse.cdt.managedbuilder.core.tests/suite/org/eclipse/cdt/managedbuilder/tests/suite/AllManagedBuildTests.java
+++ b/build/org.eclipse.cdt.managedbuilder.core.tests/suite/org/eclipse/cdt/managedbuilder/tests/suite/AllManagedBuildTests.java
@@ -33,6 +33,7 @@ import org.eclipse.cdt.managedbuilder.core.tests.OptionEnablementTests;
 import org.eclipse.cdt.managedbuilder.core.tests.PathConverterTest;
 import org.eclipse.cdt.managedbuilder.core.tests.ResourceBuildCoreTests;
 import org.eclipse.cdt.projectmodel.tests.BackwardCompatiblityTests;
+import org.eclipse.cdt.projectmodel.tests.CProjectDescriptionSerializationTests;
 import org.eclipse.cdt.projectmodel.tests.OptionStringListValueTests;
 import org.eclipse.cdt.projectmodel.tests.ProjectModelTests;
 
@@ -73,6 +74,7 @@ public class AllManagedBuildTests {
 		suite.addTest(ProjectModelTests.suite());
 		suite.addTest(OptionStringListValueTests.suite());
 		suite.addTest(BackwardCompatiblityTests.suite());
+		suite.addTest(CProjectDescriptionSerializationTests.suite());
 		//$JUnit-END$
 		return suite;
 	}
diff --git a/build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/projectmodel/tests/CProjectDescriptionSerializationTests.java b/build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/projectmodel/tests/CProjectDescriptionSerializationTests.java
new file mode 100644
index 00000000000..a2cb3fb62e7
--- /dev/null
+++ b/build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/projectmodel/tests/CProjectDescriptionSerializationTests.java
@@ -0,0 +1,141 @@
+/*******************************************************************************
+ * Copyright (c) 2005, 2007 Intel Corporation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Intel Corporation - Initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.projectmodel.tests;
+
+import junit.framework.Assert;
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestSuite;
+
+import org.eclipse.cdt.core.model.CoreModel;
+import org.eclipse.cdt.core.settings.model.ICConfigurationDescription;
+import org.eclipse.cdt.core.settings.model.ICProjectDescription;
+import org.eclipse.cdt.core.settings.model.extension.CConfigurationData;
+import org.eclipse.cdt.managedbuilder.core.IConfiguration;
+import org.eclipse.cdt.managedbuilder.core.IManagedBuildInfo;
+import org.eclipse.cdt.managedbuilder.core.IProjectType;
+import org.eclipse.cdt.managedbuilder.core.ManagedBuildManager;
+import org.eclipse.cdt.managedbuilder.internal.core.Configuration;
+import org.eclipse.cdt.managedbuilder.internal.core.ManagedBuildInfo;
+import org.eclipse.cdt.managedbuilder.internal.core.ManagedProject;
+import org.eclipse.cdt.managedbuilder.testplugin.BuildSystemTestHelper;
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IWorkspace;
+import org.eclipse.core.resources.IWorkspaceDescription;
+import org.eclipse.core.resources.IWorkspaceRoot;
+import org.eclipse.core.resources.ResourcesPlugin;
+/**
+ * TODO
+ *
+ */
+public class CProjectDescriptionSerializationTests extends TestCase {
+	/**
+	 * @return Test
+	 */
+	public static Test suite() {
+		return new TestSuite(CProjectDescriptionSerializationTests.class);
+	}
+
+	@Override
+	protected void setUp() throws Exception {
+	}
+
+	@Override
+	protected void tearDown() throws Exception {
+	}
+
+	/**
+	 * This test is intended to test serialization of C++ project
+	 * @throws Exception
+	 */
+	public void testTooManyConfigurations() throws Exception {
+		String projectName = "testTooManyConfigurations";
+		String pluginProjectTypeId = "cdt.managedbuild.target.gnu.cygwin.exe";
+
+		CoreModel coreModel = CoreModel.getDefault();
+		
+		{
+			// Create model project and accompanied descriptions
+			IProject project = BuildSystemTestHelper.createProject(projectName);
+			ICProjectDescription des = coreModel.createProjectDescription(project, false);
+			Assert.assertNotNull("createDescription returned null!", des);
+
+			{
+				ManagedBuildInfo info = ManagedBuildManager.createBuildInfo(project);
+				IProjectType type = ManagedBuildManager.getProjectType(pluginProjectTypeId);
+				Assert.assertNotNull("project type not found", type);
+
+				ManagedProject mProj = new ManagedProject(project, type);
+				info.setManagedProject(mProj);
+
+				IConfiguration cfgs[] = type.getConfigurations();
+				Assert.assertNotNull("configurations not found", cfgs);
+				Assert.assertTrue("no configurations found in the project type",cfgs.length>0);
+
+				for (IConfiguration configuration : cfgs) {
+					String id = ManagedBuildManager.calculateChildId(configuration.getId(), null);
+					Configuration config = new Configuration(mProj, (Configuration)configuration, id, false, true, false);
+					CConfigurationData data = config.getConfigurationData();
+					Assert.assertNotNull("data is null for created configuration", data);
+					ICConfigurationDescription cfgDes = des.createConfiguration(ManagedBuildManager.CFG_DATA_PROVIDER_ID, data);
+				}
+				Assert.assertEquals(2, des.getConfigurations().length);
+			}
+
+			// Persist the project
+			coreModel.setProjectDescription(project, des);
+			project.close(null);
+		}
+
+
+		{
+			IWorkspace workspace = ResourcesPlugin.getWorkspace();
+			IWorkspaceRoot root = workspace.getRoot();
+
+			IWorkspaceDescription workspaceDesc = workspace.getDescription();
+			workspaceDesc.setAutoBuilding(false);
+			workspace.setDescription(workspaceDesc);
+
+			// Trying to induce an anomaly
+			for (int i=0;i<144;i++)
+			{
+				// Open project
+				IProject project = root.getProject(projectName);
+				project.open(null);
+				Assert.assertEquals(true, project.isOpen());
+
+				// Check project description
+				ICProjectDescription des = coreModel.getProjectDescription(project);
+				Assert.assertEquals(2, des.getConfigurations().length);
+
+				IManagedBuildInfo buildInfo = ManagedBuildManager.getBuildInfo(project);
+				// once in a while managedProject.getConfigurations() can return null
+				// inside buildInfo.getConfigurationNames() which results in NPE
+				String[] configurationNames = buildInfo.getConfigurationNames();
+				// this Assert triggers as well on occasion
+				Assert.assertNotNull("buildInfo.getConfigurationNames() returned null", configurationNames);
+
+				IConfiguration configurations[] = buildInfo.getManagedProject().getConfigurations();
+				// this condition is not supposed to be true
+				// since the project is supposed to have exactly 2 configurations
+				if (configurations.length > 2) {
+					String message = i + "-th round: Too many configurations loaded. ";
+					for (IConfiguration configuration : configurations) {
+						message = message + "["+configuration.getName()+"], ";
+					}
+					Assert.assertEquals(message, 2, configurations.length);
+				}
+
+				project.close(null);
+			}
+		}
+	}
+}
diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/ManagedProject.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/ManagedProject.java
index 4ea9afa9371..849d7f01c12 100644
--- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/ManagedProject.java
+++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/ManagedProject.java
@@ -13,6 +13,7 @@ package org.eclipse.cdt.managedbuilder.internal.core;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -39,15 +40,12 @@ import org.eclipse.core.runtime.PluginVersionIdentifier;
 
 public class ManagedProject extends BuildObject implements IManagedProject, IBuildPropertiesRestriction, IBuildPropertyChangeListener {
 	
-	private static final String EMPTY_STRING = new String();
-	private static final IConfiguration[] emptyConfigs = new IConfiguration[0];
-	
 	//  Parent and children
 	private IProjectType projectType;
 	private String projectTypeId;
 	private IResource owner;
 //	private List configList;	//  Configurations of this project type
-	private Map configMap;
+	private Map<String, Configuration> configMap = Collections.synchronizedMap(new LinkedHashMap<String, Configuration>());
 	//  Miscellaneous
 	private boolean isDirty = false;
 	private boolean isValid = true;
@@ -151,8 +149,7 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 				}
 				
 				if(vars != null){
-					for(Iterator iter = getConfigurationMap().values().iterator(); iter.hasNext(); ){
-						Configuration cfg = (Configuration)iter.next();
+					for (Configuration cfg : getConfigurationCollection()) {
 						((ToolChain)cfg.getToolChain()).addProjectVariables(vars);
 					}
 				}
@@ -235,14 +232,10 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 		serializeProjectInfo(element);
 		
 		if(saveChildren){
-			Collection configElements = getConfigurationCollection();
-			Iterator iter = configElements.iterator();
-			while (iter.hasNext()) {
-				Configuration config = (Configuration) iter.next();
+			for (Configuration cfg : getConfigurationCollection()) {
 				ICStorageElement configElement = element.createChild(IConfiguration.CONFIGURATION_ELEMENT_NAME);
-				config.serialize(configElement);
+				cfg.serialize(configElement);
 			}
-			
 		}
 		// Serialize my children
 		
@@ -312,21 +305,16 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 	 * @see org.eclipse.cdt.core.build.managed.IManagedProject#getConfiguration()
 	 */
 	public IConfiguration getConfiguration(String id) {
-		return (IConfiguration)getConfigurationMap().get(id);
+		return configMap.get(id);
 	}
 	
 	/* (non-Javadoc)
 	 * @see org.eclipse.cdt.managedbuilder.core.IManagedProject#getConfigurations()
 	 */
 	public IConfiguration[] getConfigurations() {
-		IConfiguration[] configs = new IConfiguration[getConfigurationCollection().size()];
-		Iterator iter = getConfigurationCollection().iterator();
-		int i = 0;
-		while (iter.hasNext()) {
-			Configuration config = (Configuration)iter.next();
-			configs[i++] = (IConfiguration)config; 
+		synchronized (configMap) {
+			return configMap.values().toArray(new IConfiguration[configMap.size()]);
 		}
-		return configs;
 	}
 	
 	/* (non-Javadoc)
@@ -336,10 +324,10 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 		final String removeId = id;
 		
 		//handle the case of temporary configuration
-		if(getConfigurationMap().get(id) == null)
+		if(!configMap.containsKey(id))
 			return;
 
-		getConfigurationMap().remove(removeId);
+		configMap.remove(removeId);
 //
 //		IWorkspaceRunnable remover = new IWorkspaceRunnable() {
 //			public void run(IProgressMonitor monitor) throws CoreException {
@@ -402,37 +390,21 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 	 * @param Tool
 	 */
 	public void addConfiguration(Configuration configuration) {
-		if(!configuration.isTemporary()){
-//			getConfigurationList().add(configuration);
-			getConfigurationMap().put(configuration.getId(), configuration);
-		}
+		if(!configuration.isTemporary())
+			configMap.put(configuration.getId(), configuration);
 	}
 	
-	/* (non-Javadoc)
+	/** (non-Javadoc)
 	 * Safe accessor for the list of configurations.
 	 * 
 	 * @return List containing the configurations
 	 */
-	private Collection getConfigurationCollection() {
-		return getConfigurationMap().values();
-//		if (configList == null) {
-//			configList = new ArrayList();
-//		}
-//		return configList;
-	}
-	
-	/* (non-Javadoc)
-	 * Safe accessor for the map of configuration ids to configurations
-	 * 
-	 * @return
-	 */
-	public Map getConfigurationMap() {
-		if (configMap == null) {
-			configMap = new LinkedHashMap();
+	private Collection<Configuration> getConfigurationCollection() {
+		synchronized (configMap) {
+			return new ArrayList<Configuration>(configMap.values());			
 		}
-		return configMap;
 	}
-
+	
 	/*
 	 *  M O D E L   A T T R I B U T E   A C C E S S O R S
 	 */
@@ -469,11 +441,8 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 			}
 			
 			// call resolve references on any children
-			Iterator configIter = getConfigurationCollection().iterator();
-			while (configIter.hasNext()) {
-				Configuration current = (Configuration)configIter.next();
-				current.resolveReferences();
-			}
+			for (Configuration cfg : getConfigurationCollection())
+				cfg.resolveReferences();
 		}
 		return true;
 	}
@@ -495,11 +464,9 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 		
 		
 		// Otherwise see if any configurations need saving
-		Iterator iter = getConfigurationCollection().iterator();
-		while (iter.hasNext()) {
-			Configuration current = (Configuration) iter.next();
-			if (current.isDirty()) return true;
-		}
+		for (IConfiguration cfg : getConfigurationCollection())
+			if (cfg.isDirty()) 
+				return true;
 		
 		return isDirty;
 	}
@@ -510,13 +477,9 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 	public void setDirty(boolean isDirty) {
 		this.isDirty = isDirty;
 		// Propagate "false" to the children
-		if (!isDirty) {
-			Iterator iter = getConfigurationCollection().iterator();
-			while (iter.hasNext()) {
-				Configuration current = (Configuration) iter.next();
-				current.setDirty(false);
-			}		    
-		}
+		if (!isDirty)
+			for (IConfiguration cfg : getConfigurationCollection())
+				cfg.setDirty(false);
 	}
 
 	/* (non-Javadoc)
@@ -651,30 +614,30 @@ public class ManagedProject extends BuildObject implements IManagedProject, IBui
 	}
 	
 	public String[] getRequiredTypeIds() {
-		List result = new ArrayList();
+		List<String> result = new ArrayList<String>();
 		IConfiguration cfgs[] = getConfigurations();
 		for(int i = 0; i < cfgs.length; i++){
 			result.addAll(Arrays.asList(((Configuration)cfgs[i]).getRequiredTypeIds()));
 		}
-		return (String[])result.toArray(new String[result.size()]);
+		return result.toArray(new String[result.size()]);
 	}
 
 	public String[] getSupportedTypeIds() {
-		List result = new ArrayList();
+		List<String> result = new ArrayList<String>();
 		IConfiguration cfgs[] = getConfigurations();
 		for(int i = 0; i < cfgs.length; i++){
 			result.addAll(Arrays.asList(((Configuration)cfgs[i]).getSupportedTypeIds()));
 		}
-		return (String[])result.toArray(new String[result.size()]);
+		return result.toArray(new String[result.size()]);
 	}
 
 	public String[] getSupportedValueIds(String typeId) {
-		List result = new ArrayList();
+		List<String> result = new ArrayList<String>();
 		IConfiguration cfgs[] = getConfigurations();
 		for(int i = 0; i < cfgs.length; i++){
 			result.addAll(Arrays.asList(((Configuration)cfgs[i]).getSupportedValueIds(typeId)));
 		}
-		return (String[])result.toArray(new String[result.size()]);
+		return result.toArray(new String[result.size()]);
 	}
 
 	public boolean requiresType(String typeId) {