As requested by firstname.lastname@example.org in issue 50259
creating separate issue for the problem described
there: The PropertiesUtils operate under shared
global lock and call 3rd party code with/without
the lock depending on random conditions. This is
unexpected and causes deadlocks.
The main error is that by default all listeners
are notified at the end of batch updated under
readAccess, while sometimes listeners can be
notified without readAccess, while other thread is
holding write access!!!
The problem is demostrated by two tests
As previously mentioned, I don't think I agree with the suggested unit
tests, and don't know of any actual harm caused by the current
condition. Nonetheless I would like to put more safety guards on this
code just in case someone is calling it incorrectly (e.g. from
multiple threads). Not a high priority for me but should be fixed.
assert ProjectManager.mutex().isReadAccess &&
in all PropertyEvaluator callbacks, which is reasonable.
Don't exactly need so many people on CC, I think (readd yourself if
Thanks for thinking the assert is reasonable. Please notice that if
implemented, the second unit test will start to pass. As that is
exactly what the tests simulates.
The second test seems to assert that the PE change notifications will
be delivered in the same thread as the thread that wrote the
*.properties file. Which I do not guarantee - in particular, not in
the (odd) situation shown in the test.
Try to deliver changes under isReadAccess when the other thread is
under isWriteAccess. By fixing the assertion you fix this test as well.
I don't think so (haven't tried yet) - T1 acquires W, writes
something.properties, releases W, then T2 acquires R, asks for a
value, receives a change, releases R.
You are right, this might happen (after recent pnejedly fix in Mutex).
I guess this is fine, the biggest problem I could see was that one
thread is notified while the other is in W. Which would be prevented
by requiring R to notify listeners.
Right, what you mention should be prevented.
I played around with different strategies but eventually decided to
just fire the changes synchronously.
The main reason to defer them was that j2seproject's
ProjectClassPathImplementation was refiring changes whenever the
evaluator changed - see issue #47910. Turns out that when *adding* a
lot of JARs to the classpath (e.g. 100), this is not a problem - I
guess the references are added first, then javac.classpath (etc.) is
changed. But when *deleting* a lot of JARs, the references are removed
one by one, firing changes in javac.classpath each time, and this was
causing massive overhead.
If I just change PCPI to coalesce change firing in the classpath, then
it seems to be fine. Since I don't know of any particular reason why
PCPI has to fire changes synchronously (other than potential unit
tests - not yet written, apparently), this seems preferable, since it
moves the asynch quality to a higher layer where it is less likely to
Pavel B. - web/project's PCPI should perhaps be changed analogously.
A bunch of changes:
committed * Up-To-Date 1.35
committed * Up-To-Date 1.8
committed * Up-To-Date 1.8 ant/project/nbproject/project.xml
committed * Up-To-Date 1.25
committed * Up-To-Date 1.11
committed * Up-To-Date 1.26
committed * Up-To-Date 1.20
committed * Up-To-Date 1.16
committed * Up-To-Date 1.6