GNOME Bugzilla – Bug 670586
gconf-editor hits g_assertion_message in gconf_client_lookup: last_slash != NULL
Last modified: 2012-04-11 13:54:15 UTC
Since the start of the precise cycle Ubuntu gets a lot of bugs about gconf-editor asserting, i.e https://bugs.launchpad.net/ubuntu/+source/gconf-editor/+bug/912116 "#0 0x00e67416 in __kernel_vsyscall () No symbol table info available.
+ Trace 229709
opening against gconf since gconf-editor didn't change for ages one of the comment has those steps to trigger it "Reproducible (3 times in a row), at ubunty 12.04 when: 1. open ekiga 2. open gconf-editor 3. go to apps -> ekiga -> protocols -> sip 4. change value of listen_port to 50060 5. select ports from catalogue, and crash.. -------------------------- :~$ gconf-editor ** GConf:ERROR:gconf-client.c:2369:gconf_client_lookup: assertion failed: (last_slash != NULL) Aborted (core dumped)" but it's fairly easy to trigger on any key (the bug pointed before has a collection of duplicates)
Could maybe have to do with the invalid read error from https://bugzilla.gnome.org/show_bug.cgi?id=668872
Created attachment 208476 [details] [review] block "changed" signal while the model is updated That patches fixes the issue by blocking the selection changed signal while the model is being updated, with the new gtk deleting the selected row from the model leads to several changed signal emission which leads to the issue
the actual diff is: "+ /* block changed signal while the model is being updated */ + g_signal_handlers_block_by_func (gtk_tree_view_get_selection (GTK_TREE_VIEW (window->list_view)),G_CALLBACK (gconf_editor_window_update_list_selection), window); + gconf_list_model_set_root_path (GCONF_LIST_MODEL (window->list_model), path); + g_signal_handlers_unblock_by_func (gtk_tree_view_get_selection (GTK_TREE_VIEW (window->list_view)),G_CALLBACK (gconf_editor_window_update_list_selection), window); +" the patch is a bit less trivial because I moved the gconf_editor_window_selection_changed() function after gconf_editor_window_update_list_selection() to be able to block that one (could have added a definition for that function earlier in the file as well)
Just for the record, the bug started being visible this cycle due to https://bugzilla.gnome.org/show_bug.cgi?id=670882 With the new GTK selection "changed" signal are emitted when the selected row is deleted from the model and another row is selected, which leads gconf-editor to free resources it shouldn't
(In reply to comment #3) > the actual diff is: So the patch you attached is obsolete? Can you attach an updated patch?
(In reply to comment #5) > (In reply to comment #3) > > the actual diff is: > > So the patch you attached is obsolete? Can you attach an updated patch? The patch is not obsolete, it needs to add 2 lines and then move a block of code down. comment #4 just states the added 2 lines. I tried the patch, but instead of the assertion abort, I get (less frequent) segfaults instead: Program received signal SIGSEGV, Segmentation fault. 0x0000000000415a29 in gconf_list_model_get_value (tree_model=0x8ad0a0, iter= 0x7fffffffc350, column=0, value=0x7fffffffc390) at gconf-list-model.c:290 290 value_type = gconf_value->type; Missing separate debuginfos, use: debuginfo-install libudev-173-3.fc16.x86_64 libxml2-2.7.8-6.fc16.x86_64 (gdb) bt
+ Trace 230040
Created attachment 211792 [details] [review] different solution This patch avoids the crash by doing all the model row removing before doing any deleting.
Review of attachment 211792 [details] [review]: I like this approach much better if it fixes the crash. I have a comment on the code below though. ::: gconf-editor-3.0.1/src/gconf-list-model.c.orig @@ +134,3 @@ if (model->root_path != NULL) { for (list = model->values; list; list = list->next) { + gtk_tree_model_row_deleted (GTK_TREE_MODEL (model), path); I don't think this is right, at least not without doing the model->stamp increment in the same cycle.
Created attachment 211810 [details] [review] revised patch revised patch to increment model stamp before calling gtk_tree_model_row_deleted
Thanks, looks good. I now pushed this patch to git master.