Bug 156829

I've just tried the trunk build #4806 with fresh userdir and can not reproduce the issue. After "activate" mobility, I
get to the second step of the wizard which asks me to install a J2ME platform. From then I can continue the wizard till
it finishes. A J2ME project is created and no such exception thrown.

Do you have specific steps to reproduce the issue?  Which jdk is your IDE running? or the issue might have been fixed in
the latest build.

I gave the specific steps;  maybe it is windows specific?

Microsoft Windows XP [Version 5.1.2600]
java version "1.6.0_10"
Java(TM) SE Runtime Environment (build 1.6.0_10-b33)
Java HotSpot(TM) Client VM (build 11.0-b15, mixed mode, sharing)

Talk to Karol Hazerlak, as he mentioned he saw the same problem when we talked on the phone today.  I'll attach the
thread dump - sorry I forgot to do that before.

Created attachment 75847 [details]
Thread dump

Both the "Ergonomics" thread and the AWT event thread, and a Request Processor thread that loads database drivers, and
another thread that loads language support, are waiting on 0x1d25a120;  the AWT thread is holding a *lot* of locks; 
some other thread is probably directly or indirectly deadlocking with one of them:

- locked <0x1d25a120> (a org.openide.util.RequestProcessor$Task)
- locked <0x35a111b0> (a org.netbeans.modules.j2ee.deployment.impl.ServerRegistry)
- locked <0x35d0f930> (a org.netbeans.modules.server.ui.node.RootNode$ChildFactory)
- locked <0x1295b710> (a java.lang.Class for org.netbeans.modules.server.ui.node.RootNode)
- locked <0x35d0fa98> (a java.lang.Object)
- locked <0x1cb7b880> (a java.awt.Component$AWTTreeLock)

This is probably your culprit - a familiar culprit, the AWT tree lock:
"Lookup Dispatch Thread" daemon prio=2 tid=0x05a81400 nid=0x27b4 waiting for monitor entry [0x067cf0
00..0x067cfd94]
   java.lang.Thread.State: BLOCKED (on object monitor)
        at java.awt.KeyboardFocusManager.clearMostRecentFocusOwner(KeyboardFocusManager.java:1777)
        - waiting to lock <0x1cb7b880> (a java.awt.Component$AWTTreeLock)
        at java.awt.Component.setFocusable(Component.java:6728)
        at javax.swing.JSeparator.<init>(JSeparator.java:88)
        at javax.swing.JSeparator.<init>(JSeparator.java:70)
        at sun.reflect.GeneratedConstructorAccessor15.newInstance(Unknown Source)
        at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorIm
pl.java:27)
        at java.lang.reflect.Constructor.newInstance(Constructor.java:513)
        at java.lang.Class.newInstance0(Class.java:355)
        at java.lang.Class.newInstance(Class.java:308)
        at org.openide.loaders.InstanceSupport.instanceCreate(InstanceSupport.java:219)

What this suggests is that actually creating JSeparator instances in folders to define menus is very likely to construct
components off the AWT thread, and probably the real long-term fix is not to use that class to declare separators in
menu/toolbar definitions, since it is relatively common to load a bunch of actions on another thread.

Since the problem appeared with the introduction of ergonomics code, probably this issue should be refiled there, but
the JSeparator issue appears dangerous.

Note the new keyword.

Probably RootNode$ChildFactory.init should be doing its work asynch.

Reassigning to ergonomics for further evaluation. 
If anything we need to do on our part, please give a specific guideline/advice.

Please remember to use "Reassign issue to owner of selected subcomponent" whenever changing component/subcomponent.

Created attachment 75968 [details]
Here is a test showing the problem with initialization of JSeparator outside of AWT thread

I've just attached a patch to show that the code in db module is not fully correct. It can deadlock with AWT thread, 
exactly as in the originally reported thread dump. I guess the test is good enough proof to let me modify the list of 
keywords and reassign to db module maintainers. My hint: use actionsForPath instead of inventing your own way to load 
actions:
http://bits.netbeans.org/dev/javadoc/org-openide-util/org/openide/util/Utilities.html#actionsForPath(java.lang.String)

Cool.

I do wonder if we shouldn't open a separate issue for this - it would be pretty easy to create an empty class
org.openide.util.Separator and change all separator declarations to that, and log a warning when JSeparator is
encountered.  Since you can create a menu/toolbar presenter when you want, I don't think there is a real reason to
support declaring random Swing components in a layer for a menu or toolbar anyway.

Or something like
<attr name="separatorAfter" boolvalue="true"/>

Hi, I'm the guilty party, or so I thought.

jtulach, you said your test proves that the db module code that loads actions
is not fully correct, and suggested I use Utilities.actionsForPath instead of
the code that was there.  I'm fine with doing that, but when I looked at the code
for actionsForPath it was remarkably similar to the code in the db module.  In fact,
it's more than similar.  It's almost exactly the same thing.  So before I go ahead and
make the change, can you please explain how this solves the deadlock problem?

bb969818b352

No longer using JSeparator to separate DB explorer actions in the layer file.  

I don't think this is a good fix. _All_ other places in the platform and IDE which define context menus consistently use
javax.swing.JSeparator to indicate separators.

1. Utilities.actionsForPath can be changed to enumerate Lookup.Item's and avoid actually constructing JSeparator's,
which would be an improvement generally. (All code which constructs context menus from layer folders should be using
actionsForPath.) core-main #6578a110d2aa

2. As I said some time ago, org.netbeans.modules.server.ui.node.RootNode$ChildFactory.init should probably be
constructing its children incrementally in a background thread anyway.

Please assign this to the appropriate person.  I'll leave my fix in place
so that the database module is not inflicting a P1.  When this is addressed
I'll be happy to undo my changes.

phejl seems to be the author of RootNode.java. Perhaps he can consider my #2 suggestion, and reporter/reproducer can
also check whether my #6578a110d2aa (once merged to main) cures the symptom of the deadlock as well.

One way or the other, #bb969818b352 is incorrect and should be backed out.

Incorrect?  I don't agree.  I'll agree with unnecessary ;)

Using anything other than JSeparator to indicate menu separators is inconsistent with precedent and should not be
allowed in any module. In particular, ActionSeparator is an unreviewed API change (it is in a public package).

#6578a110d2aa is main so I am downgrading priority on the assumption that this fixes the bug symptom.

36a8ae3554e7 db module reverted to original code.  The deadlock is not caused by the db module.

Integrated into 'main-golden', will be available in build *200901230201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/bb969818b352
User: Rob Englander <mindstream@netbeans.org>
Log: #156829 Stop using JSeparator to separate actions in the layer file for the db explorer.

#2 fixed in main 76006fbfb331.

Integrated into 'main-golden', will be available in build *200902220201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/76006fbfb331
User: phejl@netbeans.org
Log: #156829 Deadlock creating Java ME project (init children asynchronously)