Bug #546
closedWTreeView caches index of about-to-be-removed row in response to beginRemoveRows(), and later uses the index after the row has been removed from the model
0%
Description
Hi there,
Please tell me if I've misunderstood the intended mechanics of the WAbstractItemModel. Based on the documentation, it is my understanding that before a row may be deleted from the data model, the beginRemoveRows() MUST be called so the rowsAboutToBeRemoved() signal is emitted. After doing so, the model may then proceed to remove the row, and after that MUST call endRemoveRows() to signal the rowsRemoved() event. Further, it is my understanding that the model MUST honor previously valid indexes until beginRemoveRows() has returned, and only then may go ahead and alter the model structure and with that also invalidate affected indexes. When endRemoveRows() is called, all these changes MUST be complete so the views may re-query the affected locations in the model and get the updated indexes.
If the above interpretation is the correct one, then I have encountered a bad bug with WTreeView. As the subject alludes to, the observed behavior of WTreeView is that during the beginRemoveRows() notification (or possibly earlier) it stores the Wt::WModelIndex of the about-to-be-removed row. At a later stage, well after endRemoveRows() has been called, WTreeView passes this now-very-stale Wt::WModelIndex to the model's parent() function. Depending on what is stored in the internal value/pointer of said index, this is Bad™. In the custom data model I was using when I encountered this issue, it caused very confused behavior for a while, before eventually crashing from a segmentation fault due to accessing a delete'd object.
To rule out it just being a bug in the data model I was using at the time, I have produced a minimal test application which reproduces the bad WTreeView behavior. It is attached to this bug report, and it has comments on how to use it to showcase the issue.
To reiterate the core issue here - it is, based on the available documentation, my understanding that a view is responsible for listening to the begin/end row/column removal events and invalidate any cached indexes that are affected by said removal or addition. The WTreeView does not honor this, but hangs on to the index (including internal pointer to a now deleted object), and later feeds this back into the model.
This is with Wt-3.1.5.
Files
Updated by Koen Deforche over 13 years ago
- Status changed from New to InProgress
- Assignee set to Koen Deforche
Updated by Koen Deforche over 13 years ago
Hey Johny,
You are right about how you interpret what should happen and that WTreeView does not behave to the letter of the contract between a View and a Model. I've fix this in my git copy.
The reason why this however works with built-in models is that the 'date/internal pointer' of a WModelIndex points to the internal representation of the parent, rather than the item itself (because the item itself can be identified using the row/column information within that parent, and thus this saves you in internal pointers that you need because you don't need any for leaf nodes).
I am also going to fix the reciproce problem with rowsAboutToBeInserted() and rowsInserted() [here too the current implementation of WTreeView will shift its internal indexes when rows have already been inserted, and thus will be issuing the wrong indexes]. And we will need to see if other views and proxy models also suffer (WTableView, Charts) from this bad habit.
Regards,
koen
Updated by Johny Mattsson over 13 years ago
Thank you for the speedy update Koen!
Is there any chance of seeing a new point release soon after this is fixed, or will I need to pull the git repo?
Updated by Koen Deforche over 13 years ago
- Status changed from InProgress to Resolved
Fixed in latest git now.
We are planning a 3.1.6 release before the end of the month.