diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlProjectDescriptionStorage.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlProjectDescriptionStorage.java index 2c355721078..a7ef3579cca 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlProjectDescriptionStorage.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlProjectDescriptionStorage.java @@ -813,6 +813,8 @@ public class XmlProjectDescriptionStorage extends AbstractCProjectDescriptionSto DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); Document doc = builder.newDocument(); Element newXmlEl = null; + synchronized (doc) { + synchronized (el.fLock) { if (el.fElement.getParentNode().getNodeType() == Node.DOCUMENT_NODE) { Document baseDoc = el.fElement.getOwnerDocument(); NodeList list = baseDoc.getChildNodes(); @@ -828,6 +830,7 @@ public class XmlProjectDescriptionStorage extends AbstractCProjectDescriptionSto newXmlEl = (Element) importAddNode(doc, el.fElement); } return newXmlEl; + }} } catch (ParserConfigurationException e) { throw ExceptionFactory.createCoreException(e); } catch (FactoryConfigurationError e) { diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlStorage.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlStorage.java index 0e6b914bc7a..b599f7d22f4 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlStorage.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlStorage.java @@ -34,19 +34,23 @@ import org.w3c.dom.NodeList; public class XmlStorage implements ICSettingsStorage { public static final String MODULE_ELEMENT_NAME = "storageModule"; //$NON-NLS-1$ public static final String MODULE_ID_ATTRIBUTE = "moduleId"; //$NON-NLS-1$ + // Lock to prevent concurrent access to XML DOM which isn't thread-safe for read (Bug 319245) + final Object fLock; public Element fElement; private Map fStorageElementMap = new HashMap(); - private boolean fChildrenInited; + private volatile boolean fChildrenInited; private boolean fIsReadOnly; private boolean fIsDirty; public XmlStorage(Element element, boolean isReadOnly){ fElement = element; + fLock = element.getOwnerDocument(); fIsReadOnly = isReadOnly; } public XmlStorage(InternalXmlStorageElement element) throws CoreException { fElement = element.fElement; + fLock = fElement.getOwnerDocument(); fIsReadOnly = element.isReadOnly(); element.storageCreated(this); sanityCheck(element); @@ -80,8 +84,10 @@ public class XmlStorage implements ICSettingsStorage { private void initChildren(){ if(fChildrenInited) return; - fChildrenInited = true; + synchronized (fLock) { + if (fChildrenInited) + return; NodeList children = fElement.getChildNodes(); int size = children.getLength(); for(int i = 0; i < size; i++){ @@ -98,6 +104,8 @@ public class XmlStorage implements ICSettingsStorage { createAddStorageElement(moduleId, element); } + fChildrenInited = true; + } } private InternalXmlStorageElement createAddStorageElement(String id, Element element){ @@ -127,6 +135,7 @@ public class XmlStorage implements ICSettingsStorage { removeStorage(id); // Create the storage element for import + synchronized (fLock) { Document thisDoc = fElement.getOwnerDocument(); Element newEl = thisDoc.createElement(MODULE_ELEMENT_NAME); @@ -135,6 +144,7 @@ public class XmlStorage implements ICSettingsStorage { if (el instanceof XmlStorageElement) { // If we're importing an XmlStorageElement use XML methods XmlStorageElement xmlStEl = (XmlStorageElement)el; + synchronized (xmlStEl.fLock) { Element xmlEl = xmlStEl.fElement; Document otherDoc = xmlEl.getOwnerDocument(); if(!thisDoc.equals(otherDoc)){ @@ -150,6 +160,7 @@ public class XmlStorage implements ICSettingsStorage { newEl.setAttribute(MODULE_ID_ATTRIBUTE, id); return createAddStorageElement(id, newEl); + } } else { // Otherwise importing generic ICStorageElement ICStorageElement storageEl = getStorage(id, true); @@ -159,6 +170,7 @@ public class XmlStorage implements ICSettingsStorage { storageEl.importChild(child); return storageEl; } + } } public ICStorageElement getStorage(String id, boolean create){ @@ -170,10 +182,12 @@ public class XmlStorage implements ICSettingsStorage { // throw ExceptionFactory.createIsReadOnlyException(); fIsDirty = true; + synchronized (fLock) { Document doc = fElement.getOwnerDocument(); Element child = createStorageXmlElement(doc, id); fElement.appendChild(child); se = createAddStorageElement(id, child); + } } return se; } @@ -193,6 +207,8 @@ public class XmlStorage implements ICSettingsStorage { if(fIsReadOnly) throw ExceptionFactory.createIsReadOnlyException(); + synchronized (fLock) { + synchronized (se.fLock){ fIsDirty = true; Node nextSibling = se.fElement.getNextSibling(); fElement.removeChild(se.fElement); @@ -203,6 +219,7 @@ public class XmlStorage implements ICSettingsStorage { fElement.removeChild(nextSibling); } } + }} } } diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlStorageElement.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlStorageElement.java index bda6243f6bd..148ce441030 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlStorageElement.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/xml/XmlStorageElement.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2007, 2009 Intel Corporation and others. + * Copyright (c) 2007, 2010 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 @@ -43,10 +43,12 @@ public class XmlStorageElement implements ICStorageElement { protected static final String[] emptyStringList = new String[0]; + // Lock to prevent concurrent access to XML DOM which isn't thread-safe for read (Bug 319245) + final Object fLock; public Element fElement; private ICStorageElement fParent; protected List fChildList = new ArrayList(); - private boolean fChildrenCreated; + private volatile boolean fChildrenCreated; private String[] fAttributeFilters; private String[] fChildFilters; @@ -64,7 +66,10 @@ public class XmlStorageElement implements ICStorageElement { String[] childFilters){ fElement = element; fParent = parent; - + + // Synchronize on the owner document + fLock = element.getOwnerDocument(); + if(attributeFilters != null && attributeFilters.length != 0) fAttributeFilters = attributeFilters.clone(); @@ -76,10 +81,10 @@ public class XmlStorageElement implements ICStorageElement { * Create ICStorageElement children from Xml tree */ private void createChildren(){ + synchronized (fLock) { if(fChildrenCreated) return; - fChildrenCreated = true; fChildList.clear(); NodeList list = fElement.getChildNodes(); int size = list.getLength(); @@ -90,15 +95,19 @@ public class XmlStorageElement implements ICStorageElement { createAddChild((Element)node, true, null, null); } } + fChildrenCreated = true; + } } private XmlStorageElement createAddChild(Element element, boolean alowReferencingParent, String[] attributeFilters, String[] childFilters){ + synchronized (fLock) { XmlStorageElement child = createChild(element, alowReferencingParent, attributeFilters, childFilters); fChildList.add(child); - return child; + return child; + } } protected XmlStorageElement createChild(Element element, @@ -121,6 +130,7 @@ public class XmlStorageElement implements ICStorageElement { } protected ICStorageElement[] getChildren(Class clazz, boolean load){ + synchronized (fLock) { if(load) createChildren(); @@ -128,20 +138,25 @@ public class XmlStorageElement implements ICStorageElement { clazz, fChildList.size()); return fChildList.toArray(children); + } } public ICStorageElement[] getChildrenByName(String name) { + synchronized (fLock) { createChildren(); ArrayList children = new ArrayList(); for (ICStorageElement child : fChildList) if (name.equals(child.getName())) children.add(child); return children.toArray(new ICStorageElement[children.size()]); + } } public boolean hasChildren() { + synchronized (fLock) { createChildren(); return !fChildList.isEmpty(); + } } public ICStorageElement getParent() { @@ -149,13 +164,17 @@ public class XmlStorageElement implements ICStorageElement { } public String getAttribute(String name) { + synchronized (fLock) { if(isPropertyAlowed(name) && fElement.hasAttribute(name)) return fElement.getAttribute(name); + } return null; } public boolean hasAttribute(String name) { + synchronized (fLock) { return fElement.hasAttribute(name); + } } private boolean isPropertyAlowed(String name){ @@ -188,6 +207,8 @@ public class XmlStorageElement implements ICStorageElement { for(int i = 0; i < children.length; i++){ if(children[i] == el){ XmlStorageElement xmlEl = (XmlStorageElement)el; + synchronized (fLock) { + synchronized (xmlEl.fLock) { Node nextSibling = xmlEl.fElement.getNextSibling(); fElement.removeChild(xmlEl.fElement); if (nextSibling != null && nextSibling.getNodeType() == Node.TEXT_NODE) { @@ -198,6 +219,8 @@ public class XmlStorageElement implements ICStorageElement { } } fChildList.remove(el); + } + } } } } @@ -205,16 +228,21 @@ public class XmlStorageElement implements ICStorageElement { } public void removeAttribute(String name) { + synchronized (fLock) { if(isPropertyAlowed(name)) fElement.removeAttribute(name); + } } public void setAttribute(String name, String value) { + synchronized (fLock) { if(isPropertyAlowed(name)) fElement.setAttribute(name, value); + } } public void clear(){ + synchronized (fLock) { createChildren(); ICStorageElement children[] = fChildList.toArray(new ICStorageElement[fChildList.size()]); @@ -237,6 +265,7 @@ public class XmlStorageElement implements ICStorageElement { // update the pointer node = nextChildNode; } + } } public ICStorageElement createChild(String name, @@ -245,13 +274,17 @@ public class XmlStorageElement implements ICStorageElement { String[] childFilters) { if(!isChildAlowed(name)) return null; + synchronized (fLock) { Element childElement = fElement.getOwnerDocument().createElement(name); fElement.appendChild(childElement); return createAddChild(childElement, alowReferencingParent, attributeFilters, childFilters); + } } public String getName() { + synchronized (fLock) { return fElement.getNodeName(); + } } public ICStorageElement createChild(String name) { @@ -267,6 +300,7 @@ public class XmlStorageElement implements ICStorageElement { public void setValue(String value) { Text text = getTextChild(); + synchronized (fLock) { if(value != null){ if(text == null){ text = fElement.getOwnerDocument().createTextNode(value); @@ -279,9 +313,11 @@ public class XmlStorageElement implements ICStorageElement { fElement.removeChild(text); } } + } } private Text getTextChild(){ + synchronized (fLock) { NodeList nodes = fElement.getChildNodes(); Text text = null; for(int i = 0; i < nodes.getLength(); i++){ @@ -291,8 +327,8 @@ public class XmlStorageElement implements ICStorageElement { break; } } - return text; + } } public ICStorageElement importChild(ICStorageElement el) throws UnsupportedOperationException { @@ -309,6 +345,8 @@ public class XmlStorageElement implements ICStorageElement { if(el instanceof XmlStorageElement){ XmlStorageElement xmlStEl = (XmlStorageElement)el; + synchronized (fLock) { + synchronized (xmlStEl.fLock) { Element xmlEl = xmlStEl.fElement; Document thisDoc = fElement.getOwnerDocument(); Document otherDoc = xmlEl.getOwnerDocument(); @@ -317,11 +355,10 @@ public class XmlStorageElement implements ICStorageElement { } else { xmlEl = (Element)xmlEl.cloneNode(true); } - xmlEl = (Element)fElement.appendChild(xmlEl); return createAddChild(xmlEl, alowReferencingParent, attributeFilters, childFilters); + }} } else { - // FIXME JBB allow import of other types of ICStorageElement throw new UnsupportedOperationException(); } } @@ -391,6 +428,7 @@ public class XmlStorageElement implements ICStorageElement { } public String[] getAttributeNames() { + synchronized (fLock) { NamedNodeMap nodeMap = fElement.getAttributes(); int length = nodeMap.getLength(); List list = new ArrayList(length); @@ -401,6 +439,7 @@ public class XmlStorageElement implements ICStorageElement { list.add(name); } return list.toArray(new String[list.size()]); + } } public ICStorageElement createCopy() throws UnsupportedOperationException, CoreException { @@ -411,9 +450,11 @@ public class XmlStorageElement implements ICStorageElement { protected Element createXmlElementCopy() throws CoreException { try { + synchronized (fLock) { Element newXmlEl = null; DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); Document doc = builder.newDocument(); + synchronized (doc) { if(fElement.getParentNode().getNodeType() == Node.DOCUMENT_NODE){ Document baseDoc = fElement.getOwnerDocument(); NodeList list = baseDoc.getChildNodes(); @@ -428,13 +469,9 @@ public class XmlStorageElement implements ICStorageElement { } else { newXmlEl = (Element)importAddNode(doc, fElement); } -// Document baseDoc = el.fElement.getOwnerDocument(); -// Element baseEl = baseDoc.getDocumentElement(); -// Element newXmlEl = (Element)doc.importNode(baseEl, true); - - -// doc.appendChild(newXmlEl); + } return newXmlEl; + } } catch (ParserConfigurationException e) { throw ExceptionFactory.createCoreException(e); } catch (FactoryConfigurationError e) { @@ -462,6 +499,7 @@ public class XmlStorageElement implements ICStorageElement { @Override public String toString() { StringBuilder builder = new StringBuilder(); + synchronized (fLock) { try { ByteArrayOutputStream stream = new ByteArrayOutputStream(); Transformer transformer = TransformerFactory.newInstance().newTransformer(); @@ -475,7 +513,7 @@ public class XmlStorageElement implements ICStorageElement { } catch (Exception e){ return fElement.toString(); } - + } return builder.toString(); }