GNOME Bugzilla – Bug 300089
Nested tree models (1 base + 1 sorted model using base model) causes crash when adding new rows to base model
Last modified: 2005-07-22 03:39:01 UTC
Steps to reproduce: 1. Setup nested tree models (1 base, 1 sorted) as described here http:// scentric.net/tutorial/sec-sorting.html#sec-sorting-tree-sortable. 2. Each tick of the application the treeview's selection is cleared and then the treemodel is foreach-ed to select the tree rows that should be selected according to my application (the rows correspond to selected objects in 3d space). On experimentation you have to foreach the sorted-model as its this model that is "attached" to the treeview and so you need to get an iter from this model to then gtk_tree_selection_select_iter(). I have narrowed down the crash behaviour to only occuring if i foreach the sorted-model at this step, though the crash doesn't actually happen at this point. 3. When i decide i want to add a new tree row i gtk_tree_store_append() one in.. the crash happens after adding 10-12 tree rows like this.. the addition of rows is such that the application will tick and foreach the sorted model as described in step 2) many times between adding each row (it is a user initiated action required a mouse button click to add a new object and row to the tree).. I populate the entire model twice at application start, this will all happen before the first tick foreach's the sorted model as described in 2) .. it never crashes here so its definately something having foreach-ed the model many many times (typically about 30 times a second for 10s of seconds, so the model has been foreach-ed 100s of times before it crashes). Stack trace: I am away from the machine and code right now so cannot give a full stack trace, i have identified where the crash is and will be able to append a full stack trace to this bug tomorrow. For now here is the information i have so far. The crash is happening in file gtk+-2.4.14/gtk/gtktreemodelsort.c Inside function static void gtk_tree_model_sort_row_changed (GtkTreeModel *s_model, GtkTreePath *start_s_path, GtkTreeIter *start_s_iter, gpointer data); on line 562 new_order = g_new (gint, level->array->len); It is crashing because level->array is null. I've hacked in some printfs into this function to get a rough idea of what is happening, from this i've discovered that when level gets initialised at line 492 level = iter.user_data; level->array is a valid ptr, I've determined that it gets corrupted and set to NULL somewhere between my added printfs e) and f) below printf("gtk_tree_model_sort_row_changed() e) level->array=%p\n", level->array); gtk_tree_path_up (path); gtk_tree_path_append_index (path, index); gtk_tree_model_sort_increment_stamp (tree_model_sort); printf("gtk_tree_model_sort_row_changed() f) level->array=%p\n", level->array); /* if the item moved, then emit rows_reordered */ lines 549-552. I will be able to hopefully do a memory watch debug to catch when the memory is written to once inside this function and will be able to add a full stack trace to this at the same time as i get you the proper stack trace of this crash point described here. I hope this is ok, this is the first bugzilla bug i've added and i wanted to get it into the system right away and i thought that i'd narrowed down the cause of the bug, and the location of the cause to within 3 functions even though i'd not had the foresight to get a full stack trace. Other information: I am using using gtk+-2.4.14.tar.bz2, and as described in the stack trace section I am not sure exactly where the memory corruption causing the crash is happening yet, for one i've not identified if it could be coming from callbacks that i've written (namely the treeview selection changed callbacks) a memory watch debug session should allow me to narrow down exactly where the memory corruption is occuring and if its all contained within GTK or caused by external application code that i've written.
I have managed to get some screengrabs of stack traces of the crash, and of the instruction that actually sets the ptr to null which causes the crash. I've uploaded them to here http://www.jimtreats.com/gtk/actualCrash.png http://www.jimtreats.com/gtk/memNuked.png They are from different sessions as when i do a watch point in kdbg my program failed to continue to execute correctly. I've attempted to create a test case app, which does not crash.. I must not be simulating everything that is required to replicate the bug. Thanks
What kind of things are you doing in your treeview selection changed callback? Are you modifying the treestore in any way?
It's been a good while since i had this.. Its certainly not a problem anymore but i dont know if thats cause i worked round the problem or otherwise fixed it.. i'll have a look at my code with respect to the bug when i can on Monday.
The same crash still happens in my app, only happens with nested treemodels.. I have gotten round the problem by NOT creating nested models and just using the sorted abilities of the original model. Don't know if this is bad practice but the new rows i'm adding where it is crashing i am adding to the base model. treestore = gtk_tree_store_new(NUM_COLS, G_TYPE_STRING, G_TYPE_INT); sort_model = gtk_tree_model_sort_new_with_model(GTK_TREE_MODEL(treestore)); I am adding new rows to the treestore var, not to sort_model. I tried creating a test app to recreate the problem but was unable to. "What kind of things are you doing in your treeview selection changed callback? Are you modifying the treestore in any way?" do you mean this function? gtk_tree_selection_set_select_function(selection, treeViewSelectionFunc, NULL, NULL); In that function I do nothing but gtk_tree_model_get() gtk_tree_model_iter_has_child() g_free() on the string that i got purely to analyse if the selected row was a parent and to tie up non-parent rows with the objects in the rest of my app. No modification inside that function. I have since updated gtk to gtk+-2.6.6 so its still present in fairly up to date versions. here is a new backtrace. Same as the last reported one, but in nice text and valid for 2.6.6 gtk. Program received signal SIGSEGV, Segmentation fault.
+ Trace 61667
Thread NaN (LWP 19678)
(gdb) display level 1: level = (SortLevel *) 0xca46438 (gdb) display level->array 2: level->array = (GArray *) 0x0 (gdb)
Kris, from my reading of the treemodelsort code, the problem here could be that gtk_tree_model_sort_row_changed() calls gtk_tree_model_sort_increment_stamp() somewhere in the middle, which will clear the cache, and may ultimatively call gtk_tree_model_sort_free_level() on the very level we are working with. If that happens, level->array will be NULL in line 564.
Created attachment 49405 [details] a testcase Here is a testcase. Running that triggers (testsort:14118): Gtk-CRITICAL **: gtk_tree_model_rows_reordered: assertion `new_order != NULL' failed from gtk_tree_model_sort_row_changed
Ok, the problem is that row_changed gets called on a level which has not been referenced, so the level is freed during gtk_tree_model_sort_increment_stamp(). From looking quickly at the code, I see two possible solutions: * at the top of gtk_tree_model_sort_row_changed(), check the level ref_count (and maybe also the ref counts of the elements?), if those are zero, free the level, since it's not referenced (am not really sure if this is a good idea though) * ref the node before calling _increment_stamp(), unref it afterwards, so the level doesn't get freed. I don't think moving increment_stamp() down is a good idea, since the stamp needs to be incremented before we call rows_reordered, so the level would still be freed before we actually call rows_reordered. Right now I think temporarily upping the ref_count on the element in question is the best solution. I'll attach a patch for that, and do some testing tomorrow. (It does solve the issue in the testcase though).
Created attachment 49433 [details] [review] patch adding ref/unref around increment_stamp
Kris, while I have you looking at treemodelsort, can you also check out bug 301558 ? Seems related.
Other places were we continue using a SortLevel after calling increment_stamp are gtk_tree_model_sort_sort_level() and gtk_tree_model_sort_row_deleted()
*** Bug 301558 has been marked as a duplicate of this bug. ***
I think you mean _rows_reordered() here instead of _row_deleted() (: Anyway, I think it's a good plan to also 'protect' the SortLevel here. In rows_reordered() reffing the level before _sort_sort_level and unreffing the level again after _increment_stamp() should suffice, and will protect the level from being freed in _sort_sort_level() too. If you agree with all this, I will commit the fixes tonight.
Sounds right. One thing I noticed in the patch in 301558 is that there could be a memleak: --- gtktreemodelsort.c 2004-11-30 23:45:26.000000000 +0200 +++ gtktreemodelsort.c 2005-04-25 13:21:24.000000000 +0300 @@ -814,7 +814,11 @@ gtk_tree_model_sort_increment_stamp (tree_model_sort); gtk_tree_path_free (path); if (level == tree_model_sort->root) - tree_model_sort->root = NULL; + { + /* Root level is not cleaned up in increment stamp */ + gtk_tree_model_sort_free_level(tree_model_sort, tree_model_sort->root); + tree_model_sort->root = NULL; + } return; } Have you checked that out ?
* gtk/gtktreemodelsort.c (gtk_tree_model_sort_row_changed): (gtk_tree_model_sort_sort_level): Ref the level while using it, otherwise it may get nuked by gtk_tree_model_sort_increment_stamp. (gtk_tree_model_sort_row_deleted): Don't leak the root level here. (#300089, James Bramford, Markku Vire)