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.

Bug 70914

Summary: SuiteLogicalViewTest.testModulesNode failure
Product: apisupport Reporter: Jesse Glick <jglick>
Component: ProjectAssignee: Martin Krauskopf <mkrauskopf>
Status: RESOLVED FIXED    
Severity: blocker CC: pnejedly
Priority: P2 Keywords: RANDOM, REGRESSION, TEST, THREAD
Version: 5.x   
Hardware: All   
OS: All   
Issue Type: DEFECT Exception Reporter:
Attachments: Attaching forgtotten threaddump - search for !!!! for easy reading.

Description Jesse Glick 2006-01-02 19:21:19 UTC
Your recent patch to SuiteLogicalView causes the test to fail most of the time,
because it introduces indeterminacy into the Modules node's behavior:

junit.framework.AssertionFailedError: two children expected:<2> but was:<1>
        at
org.netbeans.modules.apisupport.project.ui.SuiteLogicalViewTest.testModulesNode(SuiteLogicalViewTest.java:55)

I don't know what the patch to SLV (rev. 1.21) was trying to accomplish, since
there is no filed bug report with a thread dump. But putting the refresh into RP
is probably not a good idea. At least, it makes it impossible to test the node
correctly.
Comment 1 Martin Krauskopf 2006-01-02 22:06:24 UTC
See the attached thread dump. To reproduce:

1) try to revert it (get rid of RP)
2) create Suite and Suite Component in it
3) expand Modules node in SLV (maybe might be skipped - not sure)
3) delete Suite Component (63149) - 100% deadlock for me so far.

Other solution would be to not use ProjectManager.mutex().writeAccess() in
SuiteProject$Info.getSimpleName(). But I decided for this solution - leaving
ProjectManager.mutex().writeAccess() ASAP looked reasonable to me - something
that EQ.invokeLater()/RP.post() when manipulating GUI/setKeys() - although I
felt I'm doing something redundant. I haven't found any recommendations
regarding synchronization in our Javadoc regarding those mutex (yes after quite
short take a look).
Comment 2 Martin Krauskopf 2006-01-02 22:10:18 UTC
Created attachment 28132 [details]
Attaching forgtotten threaddump - search for !!!! for easy reading.
Comment 3 Jesse Glick 2006-01-02 23:03:18 UTC
The root cause seems to be

http://www.netbeans.org/source/browse/openide/src/org/openide/explorer/view/Attic/VisualizerNode.java?r1=1.43&r2=1.44

VisualizerNode acquires Children.MUTEX just to get a node label. Which is
strange since Children.MUTEX is supposed to control just operations on children,
not on the nodes themselves.

I would suggest at least using EventQueue.invokeLater, not
RequestProcessor.default.post, to call setKeys. That would be more easily
testable. Really the threading behavior of Children.Keys is fundamentally wrong
and all we can do is work around it until it is rewritten.
Comment 4 Marian Mirilovic 2006-01-03 09:11:17 UTC
After discussion with Martin, he described me this issue as reported against NB
5.1, so changing Version
Comment 5 Martin Krauskopf 2006-01-04 19:20:44 UTC
Currently the code which needs to be performed in separated thread take
presuambly little chunk of time. But probably better to not predict. So I would
stick with RP.post(). Also Children.Keys.setKeys() about its quickness. But then
I really (as you noticed) don't know how to test this.

Is the following correct? (sticking with RP)

  while (modulesNode.getChildren().getNodes(true).length != 2) {
      Thread.sleep(150);
  } // else xtest timeout is reached --> test fails (???)

If not I would go with EQ approach... (with patching SLV s/RP/EQ)

  EventQueue.invokeLater(new Runnable() {
      public void run() {
          assertEquals("two children", 2,
                  modulesNode.getChildren().getNodes(true).length);
      }
  });


Comment 6 Jesse Glick 2006-01-04 19:25:33 UTC
I would still suggest using EQ.invokeLater and in the test maybe use

updateDataModelHowever();
EQ.iL(new Runnable() {public void run() {}});
assert(...);
Comment 7 Martin Krauskopf 2006-01-04 20:21:53 UTC
> EQ.iL(new Runnable() {public void run() {}});

Did you mean EQ.invokeAndWait()? Otherwise I didn't get it :)

Anyway nice trick. I used EQ which allows me to test both cases. Fixed.

ui/SuiteLogicalView.java; 1.21 -> 1.22;
test/unit/ui/SuiteLogicalViewTest.java; 1.6 -> 1.7;