ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Bodewig <bode...@apache.org>
Subject Locking in Project and PropertyHelper
Date Tue, 17 Jun 2008 12:38:43 GMT
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


Mime
View raw message