[PATH] core: implement a safe wl_signal_emit

[Markus Ongyerth]

> Since a destroy signal inidicates the object is utterly dead, I don't think 
> it's unreasonable for users to have assumed that they don't have to clean up 
> their listener link.  It's *never* going to fire again, so why should 
> anything need to be removed?

This only implies that the signal is never fired.
If *any* listener that's on the signal wants to remove itself form the list, 
it will access the memory of the recently freed listener.

While this is unlikly in a codebase where the full implementation is in one 
hand, after they made the decision to handle things this way, it's possible if 
a library is used, that doesn't follow the convention.

> It seems that this patch makes that assumption invalid, and we would 
> need patches to weston, enlightenment, and mutter to prevent a 
> use-after-free during the signal emit?  Now I'm seeing valgrind errors 
> on E and weston during buffer destroy.
> 
> Personally, I don't think we should change this assumption and declare 
> the existing code that's worked for years suddenly buggy. :/

The code was buggy the whole time. Just because it was never triggered, does 
not imply it's not a bug.
free()ing these struct wl_list without removing them from the signal list 
leaves other struct wl_list that are outside the control of the current code 
in an invalid, prone to use-after-free, state.
Suddenly allowing this is a breaking API change (*some* struct wl_list inside 
a wl_listener) can suddenly become invalid for reasons outside the users 
control.

Related to this entire thing:
In [1] you added tests for this and promote something, that is in essence, a 
breaking change.

It also makes good wrapper implementations into managed languages annoying.
For example (admittedly my own) [2] ensures a wl_listener can never be lost 
and leak memory. It is freed when the Handle is GC'd.
To prevent any use-after-free into this wl_listener, it removes the listener 
from the list beforehand.
I would very much like to keep this code (since it is perfectly valid on the 
current ABI) and is good design in managed languages.

With kind regards,
ongy


[1] 
https://lists.freedesktop.org/archives/wayland-devel/2018-February/037169.html
[2] 
https://github.com/swaywm/hsroots/blob/master/src/Graphics/Wayland/Signal.hsc

P.S. sorry for format issues, or if doesn't get tagged into the thread 
properly, I wasn't subscribed to the list, when this came in and had to build 
references manually.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180222/fe6c80bb/attachment.sig>