Fix for [Bug 193503] Write access exception when saving project with ICDescriptor

core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/core/cdescriptor/tests/CDescriptorTests.java

@@ -12,6 +12,8 @@
 
 package org.eclipse.cdt.core.cdescriptor.tests;
 
+import java.util.Iterator;
+
 import junit.extensions.TestSetup;
 import junit.framework.Assert;
 import junit.framework.Test;
@@ -23,9 +25,12 @@ import org.eclipse.cdt.core.CDescriptorEvent;
 import org.eclipse.cdt.core.CProjectNature;
 import org.eclipse.cdt.core.ICDescriptor;
 import org.eclipse.cdt.core.ICDescriptorListener;
+import org.eclipse.cdt.core.ICDescriptorOperation;
 import org.eclipse.cdt.core.ICExtensionReference;
 import org.eclipse.cdt.core.ICOwnerInfo;
+import org.eclipse.cdt.core.model.CoreModel;
 import org.eclipse.cdt.core.testplugin.CTestPlugin;
+import org.eclipse.cdt.internal.core.pdom.PDOMManager;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IProjectDescription;
 import org.eclipse.core.resources.IResource;
@@ -70,6 +75,7 @@ public class CDescriptorTests extends TestCase {
 		suite.addTest(new CDescriptorTests("testProjectDataCreate"));
 		suite.addTest(new CDescriptorTests("testProjectDataDelete"));
 		suite.addTest(new CDescriptorTests("testConcurrentDescriptorCreation"));
+		suite.addTest(new CDescriptorTests("testConcurrentDescriptorCreation2"));
 
 		TestSetup wrapper = new TestSetup(suite) {
 
@@ -170,6 +176,63 @@ public class CDescriptorTests extends TestCase {
 		fLastEvent = null;
  	}
 
+	// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185930
+	// https://bugs.eclipse.org/bugs/show_bug.cgi?id=193503
+	public void testConcurrentDescriptorCreation2() throws Exception {
+		for (int i=0; i<100; ++i) {
+			PDOMManager pdomMgr= (PDOMManager)CCorePlugin.getIndexManager();
+			pdomMgr.shutdown();
+			fProject.close(null);
+			fProject.open(null);
+			pdomMgr.startup().schedule();
+			ICDescriptor desc= CCorePlugin.getDefault().getCProjectDescription(fProject, true);
+			NodeList childNodes= desc.getProjectData("testElement").getChildNodes();
+			int lengthBefore= childNodes.getLength();
+			final Throwable[] exception= new Throwable[10];
+			Thread[] threads= new Thread[10];
+			for (int j = 0; j < 10; j++) {
+				final int index= j;
+				Thread t= new Thread() {
+					public void run() {
+						try {
+							ICDescriptorOperation operation= new ICDescriptorOperation() {
+								public void execute(ICDescriptor descriptor, IProgressMonitor monitor) throws CoreException {
+									assertFalse(descriptor.getConfigurationDescription().isReadOnly());
+									try {
+										Thread.sleep(10);
+									} catch (InterruptedException exc) {
+									}
+									Element data = descriptor.getProjectData("testElement");
+									data.appendChild(data.getOwnerDocument().createElement("test"));
+									assertFalse(descriptor.getConfigurationDescription().isReadOnly());
+									descriptor.saveProjectData();
+								}};
+								CCorePlugin.getDefault().getCDescriptorManager().runDescriptorOperation(fProject, operation, null);
+						} catch (Throwable exc) {
+							exception[index]= exc;
+							exc.printStackTrace();
+						}
+					}
+				};
+				t.start();
+				threads[j] = t;
+				Thread.sleep(10);
+			}
+			for (int j = 0; j < threads.length; j++) {
+				if (threads[j] != null) {
+					threads[j].join();
+				}
+				assertNull(exception[j]);
+			}
+			desc= CCorePlugin.getDefault().getCProjectDescription(fProject, true);
+			childNodes= desc.getProjectData("testElement").getChildNodes();
+			int lengthAfter= childNodes.getLength();
+			assertEquals(threads.length, lengthAfter - lengthBefore);
+
+			fLastEvent = null;
+		}
+	}
+
 	public void testDescriptorOwner() throws Exception {
 		ICDescriptor desc = CCorePlugin.getDefault().getCProjectDescription(fProject, true);
 		ICOwnerInfo owner = desc.getProjectOwner();

core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/AbstractCExtensionProxy.java

@@ -48,7 +48,7 @@ public abstract class AbstractCExtensionProxy implements ICProjectDescriptionLis
 	private ICExtensionReference getRef(ICConfigurationDescription cfg, boolean update){
 		if(fExtPointId != null){
 			try {
-				CConfigBasedDescriptor dr = new CConfigBasedDescriptor(cfg);
+				CConfigBasedDescriptor dr = new CConfigBasedDescriptor(cfg, false);
 				ICExtensionReference[] cextensions = dr.get(fExtPointId, update);
 				if (cextensions.length > 0) {
 					return cextensions[0];

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/CConfigBasedDescriptor.java

@@ -24,6 +24,7 @@ import org.eclipse.cdt.core.settings.model.ICConfigurationDescription;
 import org.eclipse.cdt.core.settings.model.ICProjectDescription;
 import org.eclipse.cdt.core.settings.model.util.CDataUtil;
 import org.eclipse.cdt.core.settings.model.util.CExtensionUtil;
+import org.eclipse.cdt.internal.core.settings.model.CConfigurationDescriptionCache;
 import org.eclipse.cdt.internal.core.settings.model.CConfigurationSpecSettings;
 import org.eclipse.cdt.internal.core.settings.model.CProjectDescriptionManager;
 import org.eclipse.cdt.internal.core.settings.model.IInternalCCfgInfo;
@@ -94,7 +95,11 @@ public class CConfigBasedDescriptor implements ICDescriptor {
 	}
 
 	public CConfigBasedDescriptor(ICConfigurationDescription des) throws CoreException{
-		updateConfiguration(des);
+		this(des, true);
+	}
+
+	public CConfigBasedDescriptor(ICConfigurationDescription des, boolean write) throws CoreException{
+		updateConfiguration(des, write);
 	}
 	
 	public void setApplyOnChange(boolean apply){
@@ -119,11 +124,13 @@ public class CConfigBasedDescriptor implements ICDescriptor {
 	}
 	
 	private void checkApply() throws CoreException {
-		if(fApplyOnChange){
+		synchronized (CProjectDescriptionManager.getInstance()){
-			apply(false);
+			if(fApplyOnChange){
-			fIsDirty = false;
+				apply(false);
-		} else {
+				fIsDirty = false;
-			fIsDirty = true;
+			} else {
+				fIsDirty = true;
+			}
 		}
 	}
 	
@@ -155,6 +162,13 @@ public class CConfigBasedDescriptor implements ICDescriptor {
 	}
 	
 	public void updateConfiguration(ICConfigurationDescription des) throws CoreException{
+		updateConfiguration(des, true);
+	}
+
+	public void updateConfiguration(ICConfigurationDescription des, boolean write) throws CoreException{
+		if(write && des instanceof CConfigurationDescriptionCache)
+			throw new IllegalArgumentException();
+		
 		fCfgDes = des;
 		fProject = fCfgDes.getProjectDescription().getProject();
 		CConfigurationSpecSettings settings = ((IInternalCCfgInfo)fCfgDes).getSpecSettings(); 
@@ -248,13 +262,15 @@ public class CConfigBasedDescriptor implements ICDescriptor {
 	}
 
 	public Element getProjectData(String id) throws CoreException {
-		Element el = (Element)fStorageDataElMap.get(id);
+		synchronized(CProjectDescriptionManager.getInstance()){
-		if(el == null){
+			Element el = (Element)fStorageDataElMap.get(id);
-			InternalXmlStorageElement storageEl = (InternalXmlStorageElement)fCfgDes.getStorage(id, true);
+			if(el == null){
-			el = CProjectDescriptionManager.getInstance().createXmlElementCopy(storageEl);
+				InternalXmlStorageElement storageEl = (InternalXmlStorageElement)fCfgDes.getStorage(id, true);
-			fStorageDataElMap.put(id, el);
+				el = CProjectDescriptionManager.getInstance().createXmlElementCopy(storageEl);
+				fStorageDataElMap.put(id, el);
+			}
+			return el;
 		}
-		return el;
 	}
 
 	public ICOwnerInfo getProjectOwner() {
@@ -307,10 +323,12 @@ public class CConfigBasedDescriptor implements ICDescriptor {
 	}
 
 	public void saveProjectData() throws CoreException {
-		if(CProjectDescriptionManager.getInstance().getDescriptorManager().reconsile(this, fCfgDes.getProjectDescription()))
+		synchronized (CProjectDescriptionManager.getInstance()) {
-			fIsDirty = true;
+			if(CProjectDescriptionManager.getInstance().getDescriptorManager().reconsile(this, fCfgDes.getProjectDescription()))
+				fIsDirty = true;
 			
-		checkApply();
+			checkApply();
+		}
 	}
 	
 	public Map getStorageDataElMap(){

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/CConfigBasedDescriptorManager.java

@@ -7,6 +7,7 @@
  *
  * Contributors:
  * Intel Corporation - Initial API and implementation
+ * Anton Leherbauer (Wind River Systems)
  *******************************************************************************/
 package org.eclipse.cdt.internal.core;
 
@@ -30,7 +31,7 @@ import org.eclipse.cdt.core.settings.model.ICProjectDescription;
 import org.eclipse.cdt.core.settings.model.ICProjectDescriptionListener;
 import org.eclipse.cdt.core.settings.model.ICSettingObject;
 import org.eclipse.cdt.core.settings.model.extension.CConfigurationData;
-import org.eclipse.cdt.internal.core.settings.model.CConfigurationDescription;
+import org.eclipse.cdt.internal.core.settings.model.CConfigurationDescriptionCache;
 import org.eclipse.cdt.internal.core.settings.model.CConfigurationSpecSettings;
 import org.eclipse.cdt.internal.core.settings.model.CProjectDescription;
 import org.eclipse.cdt.internal.core.settings.model.CProjectDescriptionManager;
@@ -49,6 +50,7 @@ import org.eclipse.core.runtime.ISafeRunnable;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.QualifiedName;
+import org.eclipse.core.runtime.SafeRunner;
 import org.eclipse.core.runtime.Status;
 import org.w3c.dom.Element;
 
@@ -106,16 +108,16 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 
 	public void configure(IProject project, String id) throws CoreException {
 		CConfigBasedDescriptor dr;
-		if (id.equals(NULLCOwner.getOwnerID())) { //$NON-NLS-1$
+		if (id.equals(NULLCOwner.getOwnerID())) {
 			IStatus status = new Status(IStatus.ERROR, CCorePlugin.PLUGIN_ID, -1,
 					CCorePlugin.getResourceString("CDescriptorManager.exception.invalid_ownerID"), //$NON-NLS-1$
 					(Throwable)null);
 			throw new CoreException(status);
 		}
-		synchronized (this) {
+		synchronized (CProjectDescriptionManager.getInstance()) {
 			dr = findDescriptor(project, false);
 			if (dr != null) {
-				if (dr.getProjectOwner().getID().equals(NULLCOwner.getOwnerID())) { //$NON-NLS-1$
+				if (dr.getProjectOwner().getID().equals(NULLCOwner.getOwnerID())) {
 					// non owned descriptors are simply configure to the new owner no questions ask!
 					dr = updateDescriptor(project, dr, id);
 					dr.apply(true);
@@ -176,14 +178,14 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 //			throw ExceptionFactory.createCoreException("the projecty does not contain valid configurations");
 //		}
 
-		dr = updateDescriptor(project, dr, id);
+		synchronized(CProjectDescriptionManager.getInstance()){
-		dr.apply(true);
+			dr = updateDescriptor(project, dr, id);
+			dr.apply(true);
+		}
 	}
 
 	public ICDescriptor getDescriptor(IProject project) throws CoreException {
-		synchronized (CProjectDescriptionManager.getInstance()) {
+		return getDescriptor(project, true);
-			return getDescriptor(project, true);
-		}
 	}
 
 	public ICDescriptor getDescriptor(IProject project, boolean create)
@@ -209,7 +211,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 			throw new CoreException(new Status(IStatus.ERROR, CCorePlugin.PLUGIN_ID, -1, "Failed to create descriptor", null)); //$NON-NLS-1$
 		}
 
-		synchronized (dr) {
+		synchronized (CProjectDescriptionManager.getInstance()) {
 			boolean initialApplyOnChange = dr.isApplyOnChange();
 			dr.setApplyOnChange(false);
 			try {
@@ -217,9 +219,9 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 			} finally {
 				dr.setApplyOnChange(initialApplyOnChange);
 			}
-		}
 
-		dr.apply(false);
+			dr.apply(false);
+		}
 	}
 
 	public void runDescriptorOperation(IProject project,
@@ -236,6 +238,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 			throw new CoreException(new Status(IStatus.ERROR, CCorePlugin.PLUGIN_ID, -1, CCorePlugin.getResourceString("CConfigBasedDescriptorManager.2"), null)); //$NON-NLS-1$
 		}
 		
+		//create a new descriptor
 		dr = loadDescriptor((CProjectDescription)des);
 		
 		if (dr == null) {
@@ -244,7 +247,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 		
 		setOperatingDescriptor(project, dr);
 
-		synchronized (dr) {
+//		synchronized (CProjectDescriptionManager.getInstance()) {
 			dr.setApplyOnChange(false);
 			try {
 				op.execute(dr, monitor);
@@ -252,7 +255,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 			} finally {
 				clearOperatingDescriptor(project);
 			}
-		}
+//		}
 	}
 
 	private CConfigBasedDescriptor getLoaddedDescriptor(ICProjectDescription des){
@@ -293,7 +296,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 		
 		dr = getLoaddedDescriptor(des);
 		if(dr == null){
-			dr = loadDescriptor((CProjectDescription)des);
+			dr = loadDescriptor(des);
 			if(dr != null){
 				setLoaddedDescriptor(des, dr);
 			}
@@ -311,6 +314,9 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 		return des;
 	}
 
+	/*
+	 * creates a new descriptor
+	 */
 	private CConfigBasedDescriptor loadDescriptor(CProjectDescription des) throws CoreException{
 		CConfigBasedDescriptor dr = null;
 
@@ -319,6 +325,10 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 				des = (CProjectDescription)CProjectDescriptionManager.getInstance().getProjectDescription(des.getProject(), true);
 			
 			ICConfigurationDescription cfgDes = des.getDefaultSettingConfiguration();
+			if(cfgDes instanceof CConfigurationDescriptionCache){
+				des = (CProjectDescription)CProjectDescriptionManager.getInstance().getProjectDescription(des.getProject(), true);
+				cfgDes = des.getDefaultSettingConfiguration();
+			}
 	
 			
 			if(cfgDes != null){
@@ -335,7 +345,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 	}
 
 	public COwnerConfiguration getOwnerConfiguration(String id) {
-		if (id.equals(NULLCOwner.getOwnerID())) { //$NON-NLS-1$
+		if (id.equals(NULLCOwner.getOwnerID())) {
 			return NULLCOwner;
 		}
 		if (fOwnerConfigMap == null) {
@@ -395,7 +405,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 						if(des != null){
 							ICConfigurationDescription cfgDescription = des.getDefaultSettingConfiguration();
 							if(cfgDescription != null){
-								dr.updateConfiguration((CConfigurationDescription)cfgDescription);
+								dr.updateConfiguration(cfgDescription);
 								dr.setDirty(false);
 							} else {
 								setLoaddedDescriptor(des, null);
@@ -470,7 +480,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 				if(updatedCfg != null && dr != null){
 					CProjectDescription writableDes = (CProjectDescription)CProjectDescriptionManager.getInstance().getProjectDescription(event.getProject(), true);
 					ICConfigurationDescription indexCfg = writableDes.getDefaultSettingConfiguration();
-					dr.updateConfiguration((CConfigurationDescription)indexCfg);
+					dr.updateConfiguration(indexCfg);
 					dr.setDirty(false);
 				}
 				if(desEvent != null){
@@ -511,7 +521,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager {
 		}
 		for (int i = 0; i < listeners.length; i++) {
 			final int index = i;
-			Platform.run(new ISafeRunnable() {
+			SafeRunner.run(new ISafeRunnable() {
 
 				public void handleException(Throwable exception) {
 					IStatus status = new Status(IStatus.ERROR, CCorePlugin.PLUGIN_ID, -1,