Amebis/wxWidgets
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid using the already deleted model items when clearing it

Browse Source 
There is an impedance mismatch between wxDataViewCtrl API, which allows
deleting all items of the model at once, and GtkTreeView, which only
allows deleting them one by one, so bad things happen when we start
deleting the items from the GTK+ model after already having deleted them
from the wxDataViewModel, due to dereferencing the already freed
pointers.

Work around this by explicitly marking the model as being "temporarily
unsafe to use" by setting the stamp, uniquely identifying it, to 0 (and
ensuring that it's never 0 during the normal operation), and checking
for it in all functions that are called by GTK+ from inside
gtk_tree_model_row_deleted() that we call from Cleared().

The fix is not ideal, as the list of these functions was determined
empirically and could change in the future GTK+ versions, and also ugly,
but there doesn't seem to be any other way around this and at least now
Valgrind doesn't report invalid memory reads after calling
wxTreeListCtrl::DeleteAllItems() as it did before.
This commit is contained in:
Vadim Zeitlin
2018-01-24 22:05:14 +01:00
parent 0cea0a67f1
commit 27f705686d
1 changed files with 39 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
src/gtk/dataview.cpp
View File

@@ -680,7 +680,13 @@ wxgtk_tree_model_init(GTypeInstance* instance, void*)
{
GtkWxTreeModel* tree_model = GTK_WX_TREE_MODEL(instance);
tree_model->internal = NULL;
tree_model->stamp = g_random_int();
// 0 is handled specially in wxGtkTreeCellDataFunc, so don't use it as the
// stamp.
do
{
tree_model->stamp = g_random_int();
} while ( tree_model->stamp == 0 );
}
} // extern "C"
@@ -746,9 +752,18 @@ static GtkTreePath *
wxgtk_tree_model_get_path (GtkTreeModel *tree_model,
GtkTreeIter *iter)
{
GtkWxTreeModel *wxtree_model = (GtkWxTreeModel *) tree_model;
g_return_val_if_fail (GTK_IS_WX_TREE_MODEL (wxtree_model), NULL);
g_return_val_if_fail (iter->stamp == GTK_WX_TREE_MODEL (wxtree_model)->stamp, NULL);
g_return_val_if_fail (GTK_IS_WX_TREE_MODEL (tree_model), NULL);
GtkWxTreeModel *wxtree_model = GTK_WX_TREE_MODEL (tree_model);
if ( wxtree_model->stamp == 0 )
{
// The model is temporarily invalid and can't be used, see Cleared(),
// but we need to return some valid path from here -- just return an
// empty one.
return gtk_tree_path_new();
}
g_return_val_if_fail (iter->stamp == wxtree_model->stamp, NULL);
return wxtree_model->internal->get_path( iter );
}
@@ -1891,12 +1906,25 @@ bool wxGtkDataViewModelNotifier::Cleared()
GtkTreePath *path = gtk_tree_path_new_first(); // points to root
// It is important to avoid selection changed events being generated from
// here as they would reference the already deleted model items, which
// would result in crashes in any code attempting to handle these events.
wxDataViewCtrl::SelectionEventsSuppressor noSelection(m_internal->GetOwner());
// We also need to prevent wxGtkTreeCellDataFunc from using the model items
// not existing any longer, so change the model stamp to indicate that it
// temporarily can't be used.
const gint stampOrig = wxgtk_model->stamp;
wxgtk_model->stamp = 0;
int i;
for (i = 0; i < count; i++)
gtk_tree_model_row_deleted( GTK_TREE_MODEL(wxgtk_model), path );
gtk_tree_path_free( path );
wxgtk_model->stamp = stampOrig;
m_internal->Cleared();
return true;
@@ -3059,6 +3087,13 @@ static void wxGtkTreeCellDataFunc( GtkTreeViewColumn *WXUNUSED(column),
g_return_if_fail (GTK_IS_WX_TREE_MODEL (model));
GtkWxTreeModel *tree_model = (GtkWxTreeModel *) model;
if ( !tree_model->stamp )
{
// The model is temporarily invalid and can't be used, see the code in
// wxGtkDataViewModelNotifier::Cleared().
return;
}
wxDataViewRenderer *cell = (wxDataViewRenderer*) data;
wxDataViewItem item( (void*) iter->user_data );