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@(protected)
For additional commands, e-mail: dev-help@(protected)