Yeah, this is probably better. --Jason Ekstrand Reviewed-By: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> <div class="gmail_quote">On Jan 21, 2014 9:02 AM, "Neil Roberts" <<a href="mailto:neil@linux.intel.com">neil@linux.intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> It looks like the rest of the code assumes that shsurf->children_link is always a consistent list. For example, shell_surface_set_parent resets children_link to the empty list after it unlinks a child from its parent. The destroy_shell_surface code just calls wl_list_remove which leaves the list node with invalid NULL pointers. Maybe the simplest way to fix it is just to call shell_surface_set_parent in that code instead of directly manipulating the list. Here is a patch to do that. Regards, - Neil ------- >8 --------------- (use git am --scissors to automatically chop here) Subject: shell: Use shell_surface_set_parent to unlink children on destroy Previously when a shell surface is destroyed it would walk through the list of child surfaces and directly remove their children_link from the list. This would leave children_link as an invalid list with just NULL pointers for prev and next. Other parts of the code such as shell_surface_set_parent assume that children_link is always in a consistent state (ie, it is the list of sibling surfaces). destroy_shell_surface also assumes that it can unconditionally call wl_list_remove on the children_link of the surface being destroyed to unlink it from its parent if there is one. However, this crashes if the child's parent is destroyed first because it would leave the child with an invalid children_link. To fix it this patch makes it call shell_surface_set_parent(..., NULL) when unlinking the surface's children instead of directly manipulating the list because that function will reset children_link to an empty list leaving it in a consistent state. --- desktop-shell/shell.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 2f8e610..e3e2c96 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -2914,10 +2914,8 @@ destroy_shell_surface(struct shell_surface *shsurf) weston_view_destroy(shsurf->view); wl_list_remove(&shsurf->children_link); - wl_list_for_each_safe(child, next, &shsurf->children_list, children_link) { - wl_list_remove(&child->children_link); - child->parent = NULL; - } + wl_list_for_each_safe(child, next, &shsurf->children_list, children_link) + shell_surface_set_parent(child, NULL); wl_list_remove(&shsurf->link); free(shsurf); -- 1.8.4.2 _______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div>