ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Benson <gudnabr...@yahoo.com>
Subject Re: Locking in Project and PropertyHelper
Date Tue, 17 Jun 2008 14:34:30 GMT
Stefan:
  Your suggested changes on PropertyHelper sound
reasonable; I'm playing with them now.

-Matt

--- Stefan Bodewig <bodewig@apache.org> wrote:

> Hi all,
> 
> while looking into
>
https://issues.apache.org/bugzilla/show_bug.cgi?id=45194
> I realized
> that we are currently pretty inconsistent with our
> usage of locking in
> Project.
> 
> Right now we have:
> 
> * locking on the Project instance for the logging
> subsystem (add or
>   remove a BuildListener and the actual logging)
> 
> * locking on the PropertyHelper instance on all
> public PropertyHelper
>   methods
> 
> * locking on the Project instance when associating a
> task with the
>   current thread - but not when reading it
> 
> * locking of the references table when setting a
> reference, but not
>   when reading it
> 
> The first two cause the race condition noted in the
> bug report since
> the methods in PropertyHelper may log stuff while
> another thread is
> active logging something and its currently executing
> BuildListener
> tries to read property.
> 
> There are two places where we potentially call
> external (non-Ant) code
> inside of a synchronized section: while logging (a
> BuildListener) and
> while accessing properties (a PropertySetter or
> PropertyEvaluator).
> Either should be avoided, but I'm not sure we can.
> 
> We use locking for the log system for two reasons:
> locking the
> collection of listeners and setting a flag that
> allows us to detect
> recursive invocations of the logging system in order
> to avoid infinite
> loops (a BuildListener writing to System.out/err).
> 
> I suggest the following changes:
> 
> * add locking to getThreadTask, but don't use the
> project instance but
>   rather something like the threadTasks table (and
> use that in
>   registerTreadTask, of course).
> 
> * remove the locking of references completely. 
> Given we hand out full
>   control via Project.getReferences to whoever is
> there, locking the
>   write access to make the "do I need to print a
> warning? Add the
>   reference" operation atomic seems pretty useless
> to me.
> 
> * lock the listener collection in the add/remove
> listener methods,
>   in fireMessageLogged lock the listeners, clone
> them, give up the
>   lock, work on the clone
> 
> * turn the loggingMessage flag into a ThreadLocal,
> don't lock for it
>   at all.
> 
>   The last two changes allow BuildListeners to be
> executed without
>   holding any locks - this should avoid the deadlock
> in issue 45194.
> 
> * I'm not completely sure what the reasons for
> locking in
>   PropertyHelper are and whether:
> 
>   * lock the delegates collections in add and clone
> it in all the
>     other methods
> 
>   * invoke the delegate without any locks
> 
>   * lock on the property helper instance after all
> delegates have been
>     invoked
> 
>   would meet all the needs.  Matt?
> 
> Stefan
> 
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> dev-unsubscribe@ant.apache.org
> For additional commands, e-mail:
> dev-help@ant.apache.org
> 
> 



      

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message