problems sorting QAbstractItemModel without QSortFilterProxyModel

[TheRonin]

I have a custom QAbstractItemModel connected to a QTreeView. The model supports a basic two-tiered item hierarchy with nodes and sub-nodes. The code that i have attempts to sort the sub-nodes based on an internal attribute whenever they are added or updated.

for example:

Node* TreeModel::addSubNode(Node* parent)
{
  Node* subnode = new Node(Node::SubNode, parent->getId());
 
  QModelIndex parentIndex = getIndexForNode(parent);
  int position = parent->getChildCount();
 
  beginInsertRows(parentIndex, position, position);
  parent->addChild(subnode);
  endInsertRows();
 
  emit layoutAboutToBeChanged();
  parent->sortSubNodes();
  emit layoutChanged();
 
  return subnode;
}
 
bool lessThan(Node* node1, Node* node2)
{
  return node1->getValue() < node2->getValue();
}
 
void Node::sortSubNodes()
{
  qSort(children.begin(), children.end(), lessThan);
}

Node* TreeModel::addSubNode(Node* parent)
{
  Node* subnode = new Node(Node::SubNode, parent->getId());

  QModelIndex parentIndex = getIndexForNode(parent);
  int position = parent->getChildCount();

  beginInsertRows(parentIndex, position, position);
  parent->addChild(subnode);
  endInsertRows();

  emit layoutAboutToBeChanged();
  parent->sortSubNodes();
  emit layoutChanged();

  return subnode;
}

bool lessThan(Node* node1, Node* node2)
{
  return node1->getValue() < node2->getValue();
}

void Node::sortSubNodes()
{
  qSort(children.begin(), children.end(), lessThan);
}

To copy to clipboard, switch view to plain text mode 

Adding nodes and sub-nodes works fine, but when i start to remove them things start breaking down. A segfault tends to appear in beginRemoveRows with a QModelIndex with an invalid internalPointer. I'm fairly confident however that i only create nodes with pointers to valid parents. If i don't do the sorting the crash dissapears and things seem stable. Is it possible to do the sorting in this way or must i add a QSortFilterProxyModel?

Thank you for your time,
/TR

[anda_skoa]

It is definitely possible to do the sorting inside your model. Actually I would say this is normally the better approach (since your model knows more about the data it is operating on).

Without knowing more my guess would be problem in either the index() or the parent() method.

Cheers,
_

[Santosh Reddy]

IMO there could be many things which could lead to the behavior you are observing.

The first thing is that the QAbstractItemModel methods (at-least the virtual ones) are meant to work with each other in reference with QMadelIndex. Now having said that it is difficult to comment or suggest based on the code snippet you posted, as we (at least I) don't know how the the methods implementation looks like.

A segfault tends to appear in beginRemoveRows with a QModelIndex with an invalid internalPointer.

What do you mean, a segfault occurs in the block of code between beginRemoveRows() and endRemoveRows() or in a slot connected to beginRomoveRows()?

I'm fairly confident however that i only create nodes with pointers to valid parents.

This makes me assume that the pointer to the parent Node (application specific management item) is stored in any given QModelIndex's internal pointer, am I correct? Further just having pointer to a valid parent (Node) is not enough, what I mean is having the pointer to the correct parent is very essential. Further I wonder how can you be confident about creating nodes with pointers to valid parents when segfault occurs?

Is it possible to do the sorting in this way or must i add a QSortFilterProxyModel?

Yes it is definitely possible to have sorting just using QAbstractItemModel, in fact that is how Qt in-built QSortFilterProxyModel does it.

I know that my reply is not so constructive but as I see, the information is just enough to ponder about, but not enough to analyze the problem in conclusive manner. Lastly, these will be my first response check points.

1. Check beginRemoveRows() parameters, there is a potential that the parameters are not valid. (As this is where you see a segfault)
2. Indexes are created (I mean in index() method) in a fool proof manner (i.e. all possible parameters values are handled properly)
3. Are you using any persistent indexes, or storing any indexes? If yes then better review them.

[TheRonin]

Thank you both for your replies. I had hoped that the problem would have a simple solution, ie: "don't ever do sorting without a proxy model". That not being the case, I will start to transfer my code into a minimal compilable example, if the cause of the problem does not reveal itself in the process, i'll post it here.

What do you mean, a segfault occurs in the block of code between beginRemoveRows() and endRemoveRows() or in a slot connected to beginRomoveRows()?

The segfault occurs in the stack resulting from a call to the beginRemoveRows()-method. The execution goes via the QAbstractItemModelPrivate::rowsAboutToBeRemoved(.. .)-method and segfaults when a call to parent() returns a QModelIndex with an invalid internalPointer. Invalid because the cast to a Node results in an invalid node that then accesses illegal memory.

This makes me assume that the pointer to the parent Node (application specific management item) is stored in any given QModelIndex's internal pointer, am I correct? Further just having pointer to a valid parent (Node) is not enough, what I mean is having the pointer to the correct parent is very essential. Further I wonder how can you be confident about creating nodes with pointers to valid parents when segfault occurs?

Your assumption is correct. The problem i'm having is that the pointer no longer seems to point to a true Node-object, which is strange considering that whenever i create a QModelIndex i set the internalPointer to a valid Node or return an invalid index. Perhaps the model is calling the createIndex-method on its own with some other pointers but my code shouldn't be.

1. Check beginRemoveRows() parameters, there is a potential that the parameters are not valid. (As this is where you see a segfault)
2. Indexes are created (I mean in index() method) in a fool proof manner (i.e. all possible parameters values are handled properly)
3. Are you using any persistent indexes, or storing any indexes? If yes then better review them.

I will check these points first and foremost. Regarding point 3, the crash in rowsAboutToBeRemoved is doing a loop over persistent indexes. I have not done anything with these directly. Should i be modifying persistent indexes? I don't really know what they are so this could very well be an oversight on my part.

Thanks again for your replies, I hope my clarifications thus far have given some additional insigt and will be complementing with more information shortly.

best regards,
TR

[anda_skoa]

Perhaps the model is calling the createIndex-method on its own with some other pointers but my code shouldn't be.

No, QAbstractItemModel does not call createIndex itself, since as you pointed out this would be highly confusing for any implementation of that interface.

Are you passing your model directly to a Qt item view? Or could there be some other code that calls methods, probably passing "foreign" indexes?

Another common mistake that even happens to experienced model developers sometimes is to not handle the invalid model index correctly, e.g. assuming that all model indexes have an internal pointer, etc

Cheers,
_

[TheRonin]

No, QAbstractItemModel does not call createIndex itself, since as you pointed out this would be highly confusing for any implementation of that interface.

Glad to hear it!

Are you passing your model directly to a Qt item view? Or could there be some other code that calls methods, probably passing "foreign" indexes?

I am using a custom QTreeView but mostly for the purpose of inserting context menus. I cut that code out and what remained was an overloading of the rowsInserted(..)-method with a call to expandAll(), for keeping the tree expanded at all times. Out of curiousity i removed that call and the crash disappeared. Fantastic but confusing.

I ran my now fairly minimal test-program through valgrind in an attempt to identify any invalid writes that might still be there but not causing the crash now that the call to expandAll had been removed but found nothing. I'm guessing that i'm still doing something funky with index() or parent() but i'm not seeing it.

I've included the test-program i've reduced my case to in this post. If you have the time and inclination to look at it and help me try to identify the silly mistake i'm making here, i would be incredibly grateful.

To recreate the crash i'm experiencing, add a node and 10 subnodes. Select the last subnode and press the delete button until the program segfaults.

kind regards,
TR

ps: if the attached file is corrupt, try this dropbox link: https://dl.dropboxusercontent.com/u/...ew_example.zip

[TheRonin]

I would be grateful for any tips or advice on this subject. *bump*

[anda_skoa]

Any hint what to do with the UI to get it into a state where it crashes?

Randomly clicking buttons didn't seem to help :-)

Cheers,
_

[pkj]

Yeah. those tree model based bugs can get irritating. I suggest you use ModelTest to track your bug down. It helped me many a times. http://qt-project.org/wiki/Model_Test

[TheRonin]

Thanks for your help!

To recreate the crash i'm experiencing, add a node and 10 subnodes. Select the last subnode and press the delete button until the program segfaults.

I tried to use ModelTest but it doesn't give me any hints unfortunately. Honestly though, i had to modify the test first because it didn't like how the text for each item was dependent on its index. The test assumes an object will have the same text before and after its moved, which is not the case in my example. But after that slight modification, ModelTest doesn't say anything, despite the crash i'm seeing. But the fact that i had to modify ModelTest was what prompted me to ask my question here in the first place, perhaps moving items around in the model the way i am is the reason for my problems, and should be done via say, a QSortFilterProxyModel instead.

[mhennings]

This thread is 6 years old but appears in Google, so I'll hint to the probable solution for future readers.

If you move items the way you do it, you will invalidate your view's internal persistentModelIndices for selected items, current item etc. Next time such a persistent index will be fed to your model's data() function, it will segfault.

For sorting / rearranging items you need to either

beginResetModel();
sortItems();
endResetModel();

beginResetModel();
sortItems();
endResetModel();

To copy to clipboard, switch view to plain text mode 

or

for(row in all items) {
    toRow = sortRow(row); // or whatever
    beginMoveRows(row, toRow);
    moveItem(row, toRow);
    endMoveRows();
}

for(row in all items) {
    toRow = sortRow(row); // or whatever
    beginMoveRows(row, toRow);
    moveItem(row, toRow);
    endMoveRows();
}

To copy to clipboard, switch view to plain text mode