This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
[ 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 uninstalled 1.4.2_05. Started Netbeans. 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.
org.netbeans.modules.project.ui.ProjectsRootNode$ProjectChildren.stateChanged(ProjectsRootNode.java:279) calls org.openide.nodes.Children$Keys.refreshKey(Children.java:1816). It is not good to acquire lock in the listener. The refresh should be rescheduled.
Checking in projects/projectui/src/org/netbeans/modules/project/ui/ProjectsRootNode.java; /cvs/projects/projectui/src/org/netbeans/modules/project/ui/ProjectsRootNode.java,v <-- ProjectsRootNode.java new revision: 1.31; previous revision: 1.30 done
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 observed. 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 metadata code. 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 currently.)
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.