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
Near the end of the log you can see the assertion
error. Note that something - I believe it is
is calling VisualizerNode.doChildrenReordered but
passing in an apparently bogus reorder list; e.g.
 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
Created attachment 12444 [details]
Created attachment 12445 [details]
Excerpt from log file showing various VN/VC events and the assertion error (gzip format)
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.
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?
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.
Created attachment 12447 [details]
Patch to defer all resorts while AWT tree lock is held and coalesce/reschedule them to EQ
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
If you have some patch that you believe would work, I can try running
with it for a while to see if it helps.
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).
Created attachment 12448 [details]
Suggested workaround in tasklist/core
Created attachment 12450 [details]
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.
Look on these issues for more background why the sort go this way:
Anyway, the attached patch with coalescing multiple changes looks
useful for me too.
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.
Cf. issue #37846 in tasklist.
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
Clearing started status
Workaround in tasklist triggers issue #38244.
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 :-(.
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.
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.
The ProxyLookup bug links to my duplicate.
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
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.
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;
new revision: 1.62; previous revision: 1.61
Workaround in tasklist/core removed. Thank you.
Checking in src/org/netbeans/modules/tasklist/core/TaskChildren.java;
new revision: 1.18
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.
Created attachment 16425 [details]
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.
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;
new revision: 1.63; previous revision: 1.62
Checking in src/org/openide/explorer/view/VisualizerChildren.java;
new revision: 1.15; previous revision: 1.14
Checking in src/org/openide/explorer/view/VisualizerEvent.java;
new revision: 1.9; previous revision: 1.8
Checking in src/org/openide/explorer/view/VisualizerNode.java;
new revision: 1.42; previous revision: 1.41