[ BUILD # : 200408200409 ]
[ JDK VERSION : J2SE 1.5.0 ]
I've got beta 2
I had to platforms in netbeans. jdk 1.5 and jdk
1.4.2_05. I use 1.5 only for running netbeans.'
all my projects use 1.4.2 as the platform.
NetBeans was closed.
I downloaded and installed jdk 1.4.2_06 and
It complained that there are some problems.
I went to Tools/Platform Manager
and pressed add platform, after that I chose
/usr/java/j2sdk1.4.2_06 folder and pressed enter
and IDE hung.
Attaching thread dump
Created attachment 18216 [details]
Full thread dump
Petr N: please investigate and reassign
The build is quite old, anyway, the deadlock is simple:
In AWT, PlatformConvertor puts global properties, which writes to a
file under PM.mutex(write). It causes a file event, processed by web
probject which in turn ends up waiting on Children.mutex(write)
In one RP thread, there is a BrokenLinks action running, holding a
Children.mutex(write) and trying to get PM.mutex(read).
Passing to j2seproject for further evaluation.
Note: the thread dump also reveals one interresting thing:
two BrokenLinksActions running concurently, as the lock
they synchronize on is per action instance and there are two such
instances. Not sure if it is OK or not, just looks suspicious.
Created attachment 18225 [details]
Cleaned up thread dump for your convenience.
It is not good to acquire lock in the listener.
The refresh should be rescheduled.
new revision: 1.31; previous revision: 1.30
Jesse, you might want to look at this as iut looks like I get the
event under some lock and the fix should rather be called hotfix.
Reopening, it seems that the
org.netbeans.spi.project.support.ant.ProjectUtils does not behave
correctly. Will attach test (made by Jarda) which proves that.
Created attachment 18230 [details]
Patch of the PropertyUtilsTest showing the problem
Consider removing the hotfix in ProjectsRootNode when fixed.
Sorry, I don't consider the "hotfix" to be anything more than a proper
fix, and I don't agree with the proposed tests, insofar as I
understand what they are trying to test:
1. testNoFiringUnderWriteAccess - no way. The file change is fired
when it happens.
2. testFiringInRightThread - property changes are coalesced and fired
in read access. Firing them immediately is way too expensive for
performance. If you happen to ask for some value before the coalesced
changes are fired, then changes are fired right then.
ProjectsRootNode$ProjectChildren.stateChanged cannot assume what locks
are being held when it is called, and so it is not permitted to
acquire a new lock (Children.MUTEX). Once Children threading is fixed
to make it behave like a regular GUI component, this will not be an
issue since there will be no Children lock at all.
It would be very valuable to know what is the real intention of using
Mutex in PropertyUtils. Without that, one can only guess whether the
bug is in the idea or in the implementation.
After reading the code I think that the intention is to do few
modifications in batch under write access and notify other code then,
under read access. This is creditable effort, however as shown by the
tests, it is not implemented correctly.
The more important test is #2, which shows that the changes can be
fired sooner than the batch is finished and the events are delivered
without read access. This is inconsistent with the usual case when
listeners are notified at the end of writeAccess by postedReadRequests.
The case #1 is a bit patological, but shows actual problem and can be
easily fixed by using AtomicAction. I would suggest that as well.
Re. ProjectsRootNode$ProjectChildren - they are innocent (as shown by
the tests). The flaw is in PropertyUtils because they are calling
foreign code while some thread is holding write lock.
First of all, the apparent bug was apparently fixed, so please don't
reopen this unless the bug reoccurs. If you have some other issue,
open a fresh bug.
Re. #1, I still see no problem. You store a new .properties file, you
get a file change event right then. As designed.
Re. #2, perhaps I don't see what your test is trying to demonstrate,
but I don't see anything wrong. Property changes are coalesced (since
it is too costly to fire them individually) and normally notified
after a block of changes in the source providers. In fact, it is not
the property changes themselves that are too expensive, it is
recomputing the list of properties, so this is deferred as long as
possible in case more source changes come in. If however someone needs
a property before the scheduled recomputation and change firing, then
the current map is computed immediately.
I do not agree that the previous version of ProjectChildren was
"innocent". The current Children implementation acquires a lock when
you call setKeys. This is unfortunate, and I think in the future it
should be changed to run later in EQ which would make it safe, but for
now it does. Therefore you cannot call setKeys safely when there might
be some other lock held - e.g. while you are in a listener callback.
Re. "The flaw is in PropertyUtils because they are calling foreign
code while some thread is holding write lock." - what foreign code are
you referring to?
Again, if you still feel there is something wrong, open a (lower
priority) bug separately with more information.
*** Issue 50355 has been marked as a duplicate of this issue. ***
"Re. #1" - I agree with Jesse. I think there are two layers: FS layer
and Project layer. Anybody interested in project properties should
listen on Project layer and not respond to FS events so it does not
matter under which lock the FS events are fired.
"Re. #2" - it seems to me strange that getProperty() call can result
in change events being fired. All I expect is to get a value - I did
not change anything! However, once explained it starts making sense.
It is probably not possible, but could be the events (fired in this
case) be fired under Mutex downgraded to readAccess? I think that's
the state Yarda is asking for and that would make sense to me too.
Re. #2, "could be the events (fired in this case) be fired under Mutex
downgraded to readAccess?" - perhaps. Depends on whether you care
about the invariant that code which listens for events and invalidates
caches, etc., always "sees" change events "before" seeing new data via
direct query. This is sometimes important - maintaining chronological
accuracy - and sometimes not. Firing change events later would make
such clients incorrect; i.e. if your code is written to assume that
the state of the object (property evaluator, here) will not have
changed if you have not gotten change events from it, then your code
will be wrong. Of course there is a compromise; you can choose from
1. Fire events eagerly. Safe, but sometimes (as here) slow to compute.
2. Fire events lazily, but immediately if state is accessed directly.
The current impl. Safe, though has the odd property that calling a
non-mutating method can trigger delivery of a change event, as you
3. Fire events lazily and asynchronously (i.e. either a different
thread, or using something like readAccess which uses the same thread
but at an effectively unpredictable future point). Efficient, and no
events triggered by getters, but can break clients which are not
expecting this semantics.
Sorry, I may be slow, but I do not see answer to my initial question:
What is the (supposed) threading model of this class?
I am reopening, because whatever the goal is, it is definitively not
achieved. I am looking forward to share the vision of the desired state.
Re. "1. Fire events eagerly." - was it measured how slow this really
is? It the gain worth the trouble?
Again, this issue is a bug report about a deadlock; for any separate
questions it may have inspired please use a separate issue.
Again, the intended threading model for the property evaluator is none
- it should never be accessed concurrently from more than one thread,
so use a mutex if in doubt. That holds for most of the project system
Re. speed of firing events eagerly - yes, this was the original state,
until maybe a month ago. It showed up as being quite slow in a
profiler for common user actions. I changed it to the current state
and it was no longer a significant time consumer in the profiler.
(Each recalculation of a PropertyEvaluator is nontrivial operation,
since you normally have dozens of properties, many of them inherited
from System.getProperties, and they can refer to one another
recursively etc. - lots of string processing. It could perhaps be sped
up using more sophisticated algorithms, but I do not know how to do so
I do not agree that this deadlock has been fixed as the root cause is
still there, but as you requested that issue 50546 which talks just
about the root cause has been created.