[PATH] core: implement a safe wl_signal_emit

Markus Ongyerth wl at ongy.net
Thu Feb 22 14:14:41 UTC 2018

> 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 

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,


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>

More information about the wayland-devel mailing list