Skip to content

file-browser: remove dummy child last

Barnabás Pőcze requested to merge pobrn/gedit:file_browser_remove_dummy_last into master

Previously, when the virtual root that has any number of non-dummy children is removed from under gedit, depending on the order of calls to on_directory_monitor_event(), a segmentation fault could occur in GTK.

The reason for this is helpfully noted by GTK:

(gedit:71019): Gtk-CRITICAL **: 19:33:58.168: ../gtk/gtk/gtktreeview.c:6711 (validate_visible_area): assertion `has_next' failed.
There is a disparity between the internal view of the GtkTreeView,
and the GtkTreeModel.  This generally means that the model has changed
without letting the view know.  Any display from now on is likely to
be incorrect.

Due to this disparity gedit_file_browser_store_get_value() cannot succeed at a later point, effectively returning an invalid GValue to GTK, causing the crash in the end.

That disparity is caused by the fact that when a non-dummy node is removed, then model_check_dummy() is called on its parent (if there is one). Now, when model_remove_node_children() is removing the children of the virtual root, its first child will be a dummy node, and that is removed without a problem. However, the removal of the second child (the first non-dummy one) will result in a call to model_check_dummy(), which will create a new dummy node because the "original" dummy node has just been removed in the previous iteration. In the last iteration the model_check_dummy() call will see that there are no other children of the parent, and hence will insert the dummy node into the GtkTreeModel. And this extra insertion causes the disparity between GTK's and gedit's state.

Fix that by removing dummy child last. By not removing the dummy child first, it is guaranteed that model_check_dummy() will not create a new dummy node - although it will still insert it into the GtkTreeModel when removing the last non-dummy, but that is not an issue since the model_remove_node() takes care of removing the dummy node from the model shortly. It also guarantees that all children of a node will be removed after a call to model_remove_node_children(), and this makes it possible to simplify model_clear() as it no longer needs to concern itself with the leftover dummy node.

(Hopefully) fixes #356 (closed)

Merge request reports