There was indeed a possible raise condition for modification/reading restrictedDefinitions.
The modification done in initSubProject was not really a problem (normally only called in the initialization of a sub project, from a single thread), but the one done in updateRestrictedDefinition was. It apears that it may be possible to have the updateRestrictedDefinition called asynchronously from inside a parallel. This method will be called when a new componentDef is defined. (See [1] for a description of a componentDef) I also had to guard getRestrictedDefinitions, and the caller is supposed to guard the resulting List. Which is quiet anoying. Maybe the returned list should be a copy, and not the life instance. WDYT? [1] See https://issues.apache.org/bugzilla/show_bug.cgi?id=40511 2008/8/20 Gilles Scokart <[EMAIL PROTECTED]>: > I think there might be also a raise condition for the field > restrictedDefinitions. The update via addDataTypeDefinition is > guarded by the restrictedDefinitions object itself, but the read via > getRestrictedDefinitions is not (neither modification in > initSubProject). > But I'm not sure what a restricted datatype is, when they are defined, > and when they are used. Could it be done from multiple thread? > > > > 2008/8/20 <[EMAIL PROTECTED]>: >> Author: gscokart >> Date: Wed Aug 20 07:40:03 2008 >> New Revision: 687358 >> >> URL: http://svn.apache.org/viewvc?rev=687358&view=rev >> Log: >> Thread safety fix in case a macrodef/presetdef is executed in parallel to >> other type related tasks (like TypeFound conditions or starting a subproject) >> >> Modified: >> ant/core/trunk/src/main/org/apache/tools/ant/ComponentHelper.java >> >> Modified: ant/core/trunk/src/main/org/apache/tools/ant/ComponentHelper.java >> URL: >> http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/ComponentHelper.java?rev=687358&r1=687357&r2=687358&view=diff >> ============================================================================== >> --- ant/core/trunk/src/main/org/apache/tools/ant/ComponentHelper.java >> (original) >> +++ ant/core/trunk/src/main/org/apache/tools/ant/ComponentHelper.java Wed >> Aug 20 07:40:03 2008 >> @@ -65,19 +65,19 @@ >> private AntTypeTable antTypeTable; >> >> /** Map of tasks generated from antTypeTable */ >> - private Hashtable taskClassDefinitions = new Hashtable(); >> + private final Hashtable taskClassDefinitions = new Hashtable(); >> >> /** flag to rebuild taskClassDefinitions */ >> private boolean rebuildTaskClassDefinitions = true; >> >> /** Map of types generated from antTypeTable */ >> - private Hashtable typeClassDefinitions = new Hashtable(); >> + private final Hashtable typeClassDefinitions = new Hashtable(); >> >> /** flag to rebuild typeClassDefinitions */ >> private boolean rebuildTypeClassDefinitions = true; >> >> /** Set of namespaces that have been checked for antlibs */ >> - private Set checkedNamespaces = new HashSet(); >> + private final HashSet checkedNamespaces = new HashSet(); >> >> /** >> * Stack of antlib contexts used to resolve definitions while >> @@ -193,6 +193,13 @@ >> } >> >> /** >> + * @return A copy of the CheckedNamespace. >> + */ >> + private synchronized Set getCheckedNamespace() { >> + return (Set) checkedNamespaces.clone(); >> + } >> + >> + /** >> * Used with creating child projects. Each child >> * project inherits the component definitions >> * from its parent. >> @@ -200,14 +207,17 @@ >> */ >> public void initSubProject(ComponentHelper helper) { >> // add the types of the parent project >> - AntTypeTable typeTable = helper.antTypeTable; >> - for (Iterator i = typeTable.values().iterator(); i.hasNext();) { >> - AntTypeDefinition def = (AntTypeDefinition) i.next(); >> - antTypeTable.put(def.getName(), def); >> + AntTypeTable typeTable = (AntTypeTable) helper.antTypeTable.clone(); >> + synchronized (antTypeTable) { >> + for (Iterator i = typeTable.values().iterator(); i.hasNext();) { >> + AntTypeDefinition def = (AntTypeDefinition) i.next(); >> + antTypeTable.put(def.getName(), def); >> + } >> } >> // add the parsed namespaces of the parent project >> - for (Iterator i = helper.checkedNamespaces.iterator(); >> i.hasNext();) { >> - checkedNamespaces.add(i.next()); >> + Set inheritedCheckedNamespace = helper.getCheckedNamespace(); >> + synchronized (this) { >> + checkedNamespaces.addAll(inheritedCheckedNamespace); >> } >> >> // Add the restricted definitions >> @@ -581,12 +591,14 @@ >> // but this is for logging only... >> Class elementClass = o.getClass(); >> String elementClassname = elementClass.getName(); >> - for (Iterator i = antTypeTable.values().iterator(); i.hasNext();) { >> - AntTypeDefinition def = (AntTypeDefinition) i.next(); >> - if (elementClassname.equals(def.getClassName()) >> - && (elementClass == def.getExposedClass(project))) { >> - String name = def.getName(); >> - return brief ? name : "The <" + name + "> type"; >> + synchronized (antTypeTable) { >> + for (Iterator i = antTypeTable.values().iterator(); >> i.hasNext();) { >> + AntTypeDefinition def = (AntTypeDefinition) i.next(); >> + if (elementClassname.equals(def.getClassName()) >> + && (elementClass == def.getExposedClass(project))) { >> + String name = def.getName(); >> + return brief ? name : "The <" + name + "> type"; >> + } >> } >> } >> return getUnmappedElementName(o.getClass(), brief); >> @@ -1071,7 +1083,7 @@ >> return def == null ? null : def.getExposedClass(project); >> } >> >> - public boolean contains(Object clazz) { >> + public synchronized boolean contains(Object clazz) { >> boolean found = false; >> if (clazz instanceof Class) { >> for (Iterator i = values().iterator(); i.hasNext() && >> !found;) { >> @@ -1091,7 +1103,7 @@ >> * @param prefix prefix to match off >> * @return the (possibly empty) list of definitions >> */ >> - public List/*<AntTypeDefinition>*/ findMatches(String prefix) { >> + public synchronized List/*<AntTypeDefinition>*/ findMatches(String >> prefix) { >> ArrayList matches = new ArrayList(); >> for (Iterator i = values().iterator(); i.hasNext();) { >> AntTypeDefinition def = (AntTypeDefinition) (i.next()); >> >> >> > > > > -- > Gilles Scokart > -- Gilles Scokart --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
