ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles Scokart" <gscok...@gmail.com>
Subject Re: svn commit: r687358 - /ant/core/trunk/src/main/org/apache/tools/ant/ComponentHelper.java
Date Mon, 25 Aug 2008 13:26:06 GMT
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 <gscokart@gmail.com>:
> 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  <gscokart@apache.org>:
>> 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: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message