[Wayland-bugs] [Bug 100878] SIGSEGV on desktop-shell focus change

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=100878

--- Comment #4 from Pekka Paalanen <ppaalanen at gmail.com> ---
(In reply to worknesday from comment #0)
> The problem here is that the *identity* of the list is subject to change as
> you add/remove nodes -- especially wl_list_remove. If, say, you have a list
> 
>     struct wl_list *my_list; // initialized to some list with 3 elements
> 
> then you do
> 
>     wl_list_remove(my_list)

This is stricty in the land of "don't do that". You are removing the list's
head from the circular list. It is a very handy trick but rarely useful: it's
purpose is to be able to free() the list head element while leaving all the
nodes still valid for a wl_list_remove() call later. It is no longer possible
to iterate the list, so this is only useful in destruction paths.

> I think a better (robust) design is to wrap and hide the actual node
> pointers so that
>     1. we preserve the identity (pointer) of the list itself
>     2. we can call wl_list_remove multiple times

The current list API may not be very intuitive at first, but it probably cannot
be changed in libwayland at least due to stable ABI.

> Then, removing a node would be something like
> 
>     // assert that node is an element of list
>     void
>     wl_list_remove(struct wl_list *list, struct wl_list_node *node);

That is inconvenient. We use a lot the feature of wl_list_remove() that you do
not need to know in which list the element is in to just remove it. If we had
to look up the identity of the list to remove the node from it, that would lead
into a lot of new code for not much benefit. For instance, all object destroy
functions can currently simply wl_list_remove() without regard to which list
the node is in, as long as the node is initialized (does not even need to be in
a list really).

(In reply to worknesday from comment #3)
> ah, I see! So, does that mean the list itself is represented as an
> always-present dummy-node that resides in the list?

Yes, exactly. The list head is always part of the circular list, but the head
is not a list node - dereferencing the head as a node would produce a garbage
pointer. This is a design choice. To iterate a list, one always needs the list
head. A list cannot be iterated starting from an arbitrary node without knowing
the head, as one must never dereference the head.

> However, it looks like the crash occurred as a result of
> es->destroy_signal.listener_list.prev being NULL. Somehow, this list ceased
> to be circular (at least while traversing backwards)

Yes, that is a side-effect of list corruption. Something else is corrupting the
list, and you happen to see a crash in a list manipulation function. Another
very common effect from list corruption is an endless loop.


Since this report is full of discussion about wl_list design rather than an
actual bug, I would suggest to rename this bug, close it, and open a new one
concentrating on the actual crash.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-bugs/attachments/20170501/121779f8/attachment.html>