> 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.
signature.asc
Description: PGP signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
