GNOME Bugzilla – Bug 778839
todo.txt: gnome-todo segfaults when changing list color
Last modified: 2017-03-28 02:01:24 UTC
How to reproduce: 1. Create a new list with todo.txt provider 2. Open the list -> Click color chooser -> select some color Result: gnome-todo segfaults logging the following to terminal window: ** (gnome-todo:4675): CRITICAL **: gtd_task_list_get_color: assertion 'GTD_IS_TASK_LIST (list)' failed ** (gnome-todo:4675): CRITICAL **: gtd_task_list_get_tasks: assertion 'GTD_IS_TASK_LIST (list)' failed Thread 1 "gnome-todo" received signal SIGSEGV, Segmentation fault. 0x000055555556f5f5 in gtd_list_selector_grid_item__render_thumbnail ( item=0x5555564416b0) at /home/sadiq/jhbuild/checkout/gnome-todo/src/views/gtd-list-selector-grid-item.c:125 125 if (LUMINANCE (color) < 0.5) (gdb) thread apply all bt fu func_and funcs futimens future func_not funlockfile futimes func_or funlockfile@plt futimesat (gdb) thread apply all bt full
+ Trace 237154
Thread 1 (Thread 0x7ffff7f09a80 (LWP 4675))
Thanks
Created attachment 348182 [details] [review] list_selector_grid_item: Call update_thumbnail only if grid_item has valid list object Current gtd_list_selector_grid_item__update_thumbnail is being called inside gtd_list_selector_grid_item_state_flags_changed which is also called if widget is being disposed. So we check if dispose has been called earlier by checking if item has a list reference and call update thumbnail only if item->list is present.Otherwise, this causes segmentation fault at some places like updating list color, deletion of todo.tx file by user while todo is running etc.
Review of attachment 348182 [details] [review]: Only a minor nitpick. ::: src/views/gtd-list-selector-grid-item.c @@ +376,3 @@ GTK_WIDGET_CLASS (gtd_list_selector_grid_item_parent_class)->state_flags_changed (item, flags); + if(self->list) You should write a comment explaining when self->list is NULL. Nitpick: space between 'if' and '('
Created attachment 348522 [details] [review] list_selector_grid_item: Call update_thumbnail only if grid_item has valid list object Current gtd_list_selector_grid_item__update_thumbnail is being called inside gtd_list_selector_grid_item_state_flags_changed which is also called if widget is being disposed. So we check if dispose has been called earlier by checking if item has a list reference and call update thumbnail only if item->list is present.Otherwise,this causes segmentation fault at some places like updating list color, deletion of todo.tx file by user while todo is running etc.
Created attachment 348523 [details] [review] list_selector_grid_item: Call update_thumbnail only if grid_item has valid list object Currently gtd_list_selector_grid_item__update_thumbnail is being called inside gtd_list_selector_grid_item_state_flags_changed which is also called if widget is being disposed. So we check if dispose has been called earlier by checking if list_selector_grid_item has a list reference and call update thumbnail only if item->list is present.Otherwise, this causes segmentation fault at some places like updating list color, deletion of todo.txt file by user while todo is running etc.
Review of attachment 348523 [details] [review]: The commit message needs improvement. A few comments: - The module name doesn't use '_', but '-', without the gtd- prefix. So, 'selector-grid-item' - The title isn't very informative. In 10 words, what does the patch do? - The body also needs improvement. Good commit messages have 3 paragraphs: context, problem and fixup. Please reorganize the message like that. I know it's nitpicking, but good commit messages make a huge difference.
Created attachment 348571 [details] [review] list-selector-grid-item: :Don't Call update_thumbnail if list has been destroyed Rendering of list_selector_grid_item with null list reference causes segmentation fault errors. While updating thumbnail of grid_item, check is not imposed if list has been disposed. The problem is that grid_item_update_thumbnail is being called even if the list reference has been disposed.When provider is removed all the list that it stores gets deleted.after disposal of item->list, grid item widget flags changes which call the update_thumbnail which calls render_thumbnail. This should only be called if list has not been disposed as thumbnail will only be updated for non null list items in the grid. The fix is to check if the gtd-list-selector-item has a reference to valid list and if true call the gtd_list_selector_grid_item__update_thumbnail else donot call the update_thumbnail.
Review of attachment 348571 [details] [review]: LGTM (although the commit message still has some issues)
Created attachment 348851 [details] [review] list-selector-grid-item don't update thumbnail on dispose (Reattaching the patch with corrected commit message and comment) Rendering of list_selector_grid_item with NULL list reference causes segmentation fault errors. While updating thumbnail of grid_item, check is not imposed if list has been disposed. The problem is that grid_item_update_thumbnail is being called even if the list reference has been disposed.When provider is removed all the list that it stores gets deleted.after disposal of item->list, grid item widget flags changes which call the update_thumbnail which calls render_thumbnail. This should only be called if list has not been disposed as thumbnail will only be updated for non null list items in the grid. The fix is to check if the gtd-list-selector-item has a reference to valid list and if true call the gtd_list_selector_grid_item__update_thumbnail else do not call the update_thumbnail.
Thanks for the patch! Attachment 348851 [details] pushed as 63bffd1 - list-selector-grid-item don't update thumbnail on dispose