Please use the Apache issue tracking system for new NetBeans issues (https://issues.apache.org/jira/projects/NETBEANS0/issues) !!
Bug 37802 - "AssertionError: Null element in reorderer list" from VisualizerChildren.reordered when using Suggestions view
"AssertionError: Null element in reorderer list" from VisualizerChildren.reor...
Status: CLOSED FIXED
Product: platform
Classification: Unclassified
Component: Outline&TreeTable
3.x
PC Linux
: P2 (vote)
: 4.x
Assigned To: _ tboudreau
issues@platform
: PERFORMANCE, RANDOM, THREAD
Depends on: 35833 37846
Blocks: 33281 38273
  Show dependency treegraph
 
Reported: 2003-12-05 20:54 UTC by Jesse Glick
Modified: 2008-12-22 18:16 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
Diagnostic patch (4.99 KB, patch)
2003-12-05 20:55 UTC, Jesse Glick
Details | Diff
Excerpt from log file showing various VN/VC events and the assertion error (gzip format) (83.01 KB, application/octet-stream)
2003-12-05 20:55 UTC, Jesse Glick
Details
Patch to defer all resorts while AWT tree lock is held and coalesce/reschedule them to EQ (2.10 KB, patch)
2003-12-05 23:09 UTC, _ tboudreau
Details | Diff
Suggested workaround in tasklist/core (2.25 KB, patch)
2003-12-06 01:15 UTC, Jesse Glick
Details | Diff
Better patch (2.16 KB, patch)
2003-12-06 10:54 UTC, _ tboudreau
Details | Diff
Stack trace (1.09 KB, text/plain)
2004-07-24 00:18 UTC, Jesse Glick
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2003-12-05 20:54:26 UTC
I use the tasklist's Suggestions view (with PMD,
Javadoc provider, etc.) on a daily basis. I have
been periodically getting this assertion error -
seems to be pretty random, but not uncommon (maybe
once per hour of IDE use at least).

Attaching a diagnostic patch to Visualizer code
that tries to track it down. And attaching a
trimmed ide.log (gzipped since it is so large)
showing everything interesting that happened to
the Suggestions root node from the perspective of
VisualizerChildren.

Near the end of the log you can see the assertion
error. Note that something - I believe it is
TreeTableView.SortedNodeTreeModel.sortChildren -
is calling VisualizerNode.doChildrenReordered but
passing in an apparently bogus reorder list; e.g.
[0] when there are >1 elements in the children
list. I am guessing that this is a threading
problem, i.e. that multiple reorder events are
getting thrown at the VC and it is processing them
sequentially, so that it is getting obsolete events.

Anyway it is not clear to me why the tree table is
calling VN.dCR over and over, or even at all. (I
never change the sort order in the Suggestions
view, BTW - this is just what happens when the
suggestions are rescanned.)

You can try the patch live and see that even a
single rescan of the suggestions list causes
dozens of reorder events to be sent to the VN/VC.
Which is odd, since probably *no reorderings of
children* ever occur under this node - when it
rescans, it just removes old children and adds new
ones. Seems clear that something in TTV and/or
VN+VC is wildly broken and causing a huge storm of
useless events to be fired, probably causing
performance problems.
Comment 1 Jesse Glick 2003-12-05 20:55:12 UTC
Created attachment 12444 [details]
Diagnostic patch
Comment 2 Jesse Glick 2003-12-05 20:55:51 UTC
Created attachment 12445 [details]
Excerpt from log file showing various VN/VC events and the assertion error (gzip format)
Comment 3 _ tboudreau 2003-12-05 21:31:05 UTC
Pasting in from an email from Ivan Solemanipour, who's encountered
similar things in the debugger:

"Ok, I'll take your word for it now, until you answer my last question
below, but the performance problems I've had with TTV's are in a
slightly different area. For example ...
	- If I have a node with 5 props and I set them all in sequence each
one will fire a property change and cause a refresh of a row.

	- some of prop changes will cause a resort. If I apply changes to N
nodes I get N resorts. In general it seems like asome sort of
"batching" facility would be useful, at the level of queueing of prop
changes but that's pretty hairy stuff. The next best thing would be to
have a flag to keep the view from refreshing itself until "we're done
adjusting the world". This would be particularly useful in places
where we blow away a lot of nodes and re-create differentones (like
the locals view)."

---------------------

Seems like a related problem.  I have no idea why the resorting is
happening, since I haven't dug below the renderer level of the
TreeTableView yet (nor am I likely to for at least a month, maybe more).

There is a lot of weird cross-translation of events.  Cc'ing Ivan and
Jirka in case there are any insights.
Comment 4 ivan 2003-12-05 21:41:17 UTC
Hmmm, I know that the debugger sorts breakpoints even if the
summary column is set to unsorted. That's because the "natural order"
isn't considered natural enough. 

It stands to reason that if the Name (primary sort key) is changed
that a resort needs to occur?
Comment 5 _ tboudreau 2003-12-05 23:08:11 UTC
A vaguely possible solution - probably more of a hack, and might not
even work.  If you could hold the AWT tree lock for the duration of
the batched change operations, I could probably defer all resorts
until after the lock is released, and coalesce any duplicates.  And
holding the lock would defer any repaints as well.  Whether it would
cause some kind of deadlock is a totally different question.

Anyway, I'll attach a slightly weird patch to try to do this.  Let me
know if there's something obviously wrong with the idea - there might be.
Comment 6 _ tboudreau 2003-12-05 23:09:27 UTC
Created attachment 12447 [details]
Patch to defer all resorts while AWT tree lock is held and coalesce/reschedule them to EQ
Comment 7 Jesse Glick 2003-12-06 00:13:44 UTC
Uh, wouldn't you have to actually post DelayedSorter into EQ or RP or
something? The patch doesn't look like it would do anything.

I'm not sure this is the right approach, anyway... the method calls
from TTV are just wrong, apparently. Calling
VisualizerNode.doChildrenReordered seems to be the root of the evil.
Perhaps this method should be removed (or made private) and
TreeTableView should be rewritten to sort nodes more safely - e.g. by
creating a filter node tree to surround the regular nodes, which
automatically sorts children according to the Nodes API. This would
ensure that VisualizerChildren would get normal node events just like
any other view.

I am making this issue depend on issue #35833 since in that branch all
of this insane garbage is deleted and replaced by a simple
straightforward TreeModel based directly on Node's. Unfortunately I
don't think it is possible to merge that stuff from the branch, since
it depends on Children being updated only in EQ, which is not the case
in the trunk.

A possible workaround is in the tasklist/suggestions module - tasklist
could ensure that all of its node operations occur in EQ (it is a good
idea, even though the Nodes API does not currently require it). I am
guessing that one factor in the bug is that insert/remove requests are
coming from RP (being replanned to EQ by the visualizer layer) but TTV
(running only in EQ I hope) is frequently fooling around with the raw
VisualizerChildren list without going through Children.MUTEX. VC is,
AFAIK, not designed to handle this - it is expecting to be an EQ-only
mirror of an model (Children) which may be changing at any moment on
another thread.

If you have some patch that you believe would work, I can try running
with it for a while to see if it helps.
Comment 8 Jesse Glick 2003-12-06 01:13:56 UTC
FWIW I made a trivial patch to tasklist-core.jar that I am running
with, in line with my earlier comments, and so far I have not seen the
error. So maybe that is enough as a workaround, until #35833 can be
finished - i.e. require that anyone using TTV use only EQ for Children
mutation methods (e.g. C.K.setKeys).
Comment 9 Jesse Glick 2003-12-06 01:15:38 UTC
Created attachment 12448 [details]
Suggested workaround in tasklist/core
Comment 10 _ tboudreau 2003-12-06 10:54:50 UTC
Created attachment 12450 [details]
Better patch
Comment 11 _ tboudreau 2003-12-06 10:55:53 UTC
Jesse, you're right - when I rewrote that patch a bit before I
attached it, I left out actually posting the sort event into the EQ.

Not that it's actually necessary to use the AWT tree lock - it could
be any lock.
Comment 12 Jiri Rechtacek 2003-12-08 16:39:22 UTC
Look on these issues for more background why the sort go this way:
http://www.netbeans.org/issues/show_bug.cgi?id=31698 and
http://www.netbeans.org/issues/show_bug.cgi?id=32328.
Anyway, the attached patch with coalescing multiple changes looks
useful for me too.
Comment 13 Jesse Glick 2003-12-08 16:46:33 UTC
Have not yet tried Tim's revised patch. Anyway the patch to
tasklist-core.jar I attached is working fine for me. So I will open a
separate patch item for this - it would anyway be desirable under #35833.

Tim, do you want me to test your revised patch for a while *without*
the tasklist patch to see if it actually does anything? It doesn't
look to me like it addresses the problem as I understand it, which is
that (1) VN.dCR is inherently unsafe to call since it can be out of
order w.r.t. real NodeReorderEvent's, (2) TTV should not be sorting at
the level of VC anyway since VC is not designed to handle it. The
patch might be useful for other reasons not related to this bug, e.g.
efficiency as Ivan suggested.
Comment 14 Jesse Glick 2003-12-08 16:49:08 UTC
Cf. issue #37846 in tasklist.
Comment 15 _ tboudreau 2003-12-10 17:54:37 UTC
Marking this P3 - for one thing, a workaround is possible, for
another, until there's time to do a serious rewrite, I'm not going to
spend a lot of time putting still more band-aids and chewing gum on
TreeTableView.
Comment 16 _ tboudreau 2003-12-21 14:52:10 UTC
Clearing started status
Comment 17 _ pkuzel 2004-01-06 16:15:09 UTC
Workaround in tasklist triggers issue #38244. 
Comment 18 _ pkuzel 2004-04-29 10:55:20 UTC
Tim, could you fix till promo-D. AWT workaround causes reponsiveness
problems. Setting keys touches lookup. On lookup there are listening
projects actions. All done synchronously from AWT => UI freeze :-(.
Comment 19 _ tboudreau 2004-04-29 12:52:10 UTC
I can't say better than maybe.  I hoped (as you know) to rewrite TTV for promo D, but my 
time has been scheduled for other things.

I intend to branch and start an attempt to integrate contrib/ttv this weekend.  If I make a 
lot of progress I might be able to give you something (delivered as a separate module) that 
will work.  But don't hold your breath - there are other things on my plate.
Comment 20 Jesse Glick 2004-04-29 15:35:49 UTC
Petr can you be more specific about the project actions? It should be
fine to be updating the keys in EQ and having actions react to that
synchronously. We have a report of a performance bug in ProxyLookup (I
think) which is under investigation and should be solved. If you have
any other information, please file a separate bug report with details.
Comment 21 _ pkuzel 2004-04-29 16:07:09 UTC
The ProxyLookup bug links to my duplicate.
Comment 22 _ ttran 2004-06-15 20:23:57 UTC
How bad is this bug?  Does it appear in NB standard build?

If it's P2 we can't easily mark the TM to "future".  To release we
have to fix all P1s and most of P2s
Comment 23 _ tboudreau 2004-06-15 21:03:20 UTC
My understanding from Jirka is that this is the result of a quick hack to fix some problem 
just before 3.4 or 3.5 was released.  I'm sure resorting the children constantly does 
nothing nice for performance, not to mention that VisualizerNode is not really meant to be 
abused this way.

At the same time, I'd prefer not to invest a lot of time into the existing TTV codebase, 
given that we are rewriting it and the odds that any fix will cause new bugs are quite high.
Comment 24 _ tboudreau 2004-07-21 23:19:06 UTC
Fixed - at least, with the tasklist workaround reverted, it is unreproducible.  What I did was 
simply coalesce all resorts and run them later on the EQ, so the underlying Node is at least 
much more likely to be in a reasonable state vis-a-vis the VisualizerChildren.  It is of 
course still possible for some other subsequent change happening on a different thread as 
the queued stuff runs to cause the problem to recur, but much lower probability;  we'll fix 
this properly in the TTV rewrite.

Checking in src/org/openide/explorer/view/TreeTableView.java;
/cvs/openide/src/org/openide/explorer/view/TreeTableView.java,v  <--  
TreeTableView.java
new revision: 1.62; previous revision: 1.61
done
Comment 25 _ pkuzel 2004-07-22 10:05:17 UTC
Workaround in tasklist/core removed. Thank you.

Checking in src/org/netbeans/modules/tasklist/core/TaskChildren.java;
new revision: 1.18
Comment 26 Jesse Glick 2004-07-24 00:18:07 UTC
Nope, happened to me in 040723, 1.5.0 b58, GTK. Had tasklist window
open, showing all todos in ant/freeform. Clicked on Current File (with
ClasspathPanelTest.java open in the editor). Showed apparently correct
results but threw this exception. Not reproducible.
Comment 27 Jesse Glick 2004-07-24 00:18:51 UTC
Created attachment 16425 [details]
Stack trace
Comment 28 _ tboudreau 2004-07-24 03:25:46 UTC
Dang!

Well, I'll bang on it some more, but if it's risky, the workaround may have to go back in.  
You're right that the correct way to do this would be filternodes to do the ordering, but 
that would be a pretty massive change to a soon-to-die codebase.
Comment 29 _ tboudreau 2004-07-25 03:16:01 UTC
Should really be fixed this time - the problem codepath is gone completely; instead, we 
just pass the Comparator to the VisualizerNode and let it use it to sort its children.  
Heaven knows why it wasn't done this way to begin with.

Checking in src/org/openide/explorer/view/TreeTableView.java;
/cvs/openide/src/org/openide/explorer/view/TreeTableView.java,v  <--  
TreeTableView.java
new revision: 1.63; previous revision: 1.62
done
Checking in src/org/openide/explorer/view/VisualizerChildren.java;
/cvs/openide/src/org/openide/explorer/view/VisualizerChildren.java,v  <--  
VisualizerChildren.java
new revision: 1.15; previous revision: 1.14
done
Checking in src/org/openide/explorer/view/VisualizerEvent.java;
/cvs/openide/src/org/openide/explorer/view/VisualizerEvent.java,v  <--  
VisualizerEvent.java
new revision: 1.9; previous revision: 1.8
done
Checking in src/org/openide/explorer/view/VisualizerNode.java;
/cvs/openide/src/org/openide/explorer/view/VisualizerNode.java,v  <--  
VisualizerNode.java
new revision: 1.42; previous revision: 1.41
done
Comment 30 Marian Mirilovic 2005-11-01 09:18:56 UTC
closed


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo