From 63d156caf355cf40cb0dae0010587c3501a1a5a8 Mon Sep 17 00:00:00 2001 From: Mikhail Sennikovsky Date: Thu, 5 Jul 2007 17:30:14 +0000 Subject: [PATCH] Fix for [Bug 193503] Write access exception when saving project with ICDescriptor --- .../cdescriptor/tests/CDescriptorTests.java | 63 +++++++++++++++++++ .../model/AbstractCExtensionProxy.java | 2 +- .../internal/core/CConfigBasedDescriptor.java | 54 ++++++++++------ .../core/CConfigBasedDescriptorManager.java | 50 +++++++++------ 4 files changed, 130 insertions(+), 39 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/core/cdescriptor/tests/CDescriptorTests.java b/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/core/cdescriptor/tests/CDescriptorTests.java index e4267880647..bafdfaf3c65 100644 --- a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/core/cdescriptor/tests/CDescriptorTests.java +++ b/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(); diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/AbstractCExtensionProxy.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/AbstractCExtensionProxy.java index 0fcde978f3d..651ae9f5a4e 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/AbstractCExtensionProxy.java +++ b/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]; diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/CConfigBasedDescriptor.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/CConfigBasedDescriptor.java index e674a904180..1dde988d501 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/CConfigBasedDescriptor.java +++ b/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; @@ -92,9 +93,13 @@ 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){ - apply(false); - fIsDirty = false; - } else { - fIsDirty = true; + synchronized (CProjectDescriptionManager.getInstance()){ + if(fApplyOnChange){ + apply(false); + fIsDirty = false; + } else { + fIsDirty = true; + } } } @@ -153,8 +160,15 @@ public class CConfigBasedDescriptor implements ICDescriptor { void setDirty(boolean dirty){ fIsDirty = dirty; } - + 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); - if(el == null){ - InternalXmlStorageElement storageEl = (InternalXmlStorageElement)fCfgDes.getStorage(id, true); - el = CProjectDescriptionManager.getInstance().createXmlElementCopy(storageEl); - fStorageDataElMap.put(id, el); + synchronized(CProjectDescriptionManager.getInstance()){ + Element el = (Element)fStorageDataElMap.get(id); + if(el == null){ + InternalXmlStorageElement storageEl = (InternalXmlStorageElement)fCfgDes.getStorage(id, true); + 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())) - fIsDirty = true; - - checkApply(); + synchronized (CProjectDescriptionManager.getInstance()) { + if(CProjectDescriptionManager.getInstance().getDescriptorManager().reconsile(this, fCfgDes.getProjectDescription())) + fIsDirty = true; + + checkApply(); + } } public Map getStorageDataElMap(){ diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/CConfigBasedDescriptorManager.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/CConfigBasedDescriptorManager.java index 68752886d68..050865e2089 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/CConfigBasedDescriptorManager.java +++ b/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); - dr.apply(true); + synchronized(CProjectDescriptionManager.getInstance()){ + 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){ @@ -279,7 +282,7 @@ public class CConfigBasedDescriptorManager implements ICDescriptorManager { } return dr; } - + private CConfigBasedDescriptor findDescriptor(CProjectDescription des) throws CoreException{ CConfigBasedDescriptor dr = getApplyingDescriptor(des.getProject()); if(dr != null) @@ -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,