wlr_* structs should be free'd when the wl_resource is

[emersion]

Rationale: it shouldn't be possible to destroy a wlr_* struct without destroying its resource.

Right now, we generally have a wlr_thing_destroy that destroys the wlr_thing without destroying the resource. Then, we have a wl_resource destroy handler which calls wlr_thing_destroy. This is an issue since calling wlr_thing_destroy won't free the resource (ie. leak memory, and more importantly allow the client to send requests to it and make the compositor crash).

We can't call wl_resource_destroy from wlr_thing_destroy because we can't call wl_resource_destroy from the resource destroy handler. We also still want to destroy the wlr_thing if the client disconnects.

So instead we should make wlr_thing_destroy just call wl_resource_destroy (if we want to keep this function at all), and the resource destroy handler do the actual cleaning.

TODO:

• [x] Core Wayland protocol
• [ ] gamma control
• [ ] idle inhibit
• [ ] idle
• [x] input inhibitor
• [ ] layer shell
• [x] linux dmabuf
• [ ] primary selection
• [x] ~~screenshooter~~ will be replaced
• [x] ~~server decoration~~ will be replaced
• [x] xdg output
• [ ] zxdg-shell v6, xdg-shell stable

---

wlroots has migrated to gitlab.freedesktop.org. This issue has been moved to:

https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/909

[ddevault]

So instead we should make wlr_thing_destroy just call wl_resource_destroy (if we want to keep this function at all), and the resource destroy handler do the actual cleaning.

I've tried to do this before and ran into Problems

[emersion]

I've tried to do this before and ran into Problems

Allowing clients to crash the compositor is a Big Problem. I guess I'll try to do this on one interface, try to reproduce your Problems, and if I can't go on.

I'll have a look at Weston's strategy to deal with this, not sure it's applicable to wlroots though.

[Ongy]

Do you remember which problems? I've done it this way in my tablet-v2 branch, and didn't run into any problems yet.

But on that one most (all) structs with resources are hidden from the compositor, and it only interacts with "multiplexer" structs, that have lists of client bindings (kinda like wlr_seat)

[ddevault]

I'm afraid I don't remember the exact semantics, but if you use this approach you should test it very carefully and try to hit every edge case.

[emersion]

Okay, I now remember the reason why this doesn't always work. Wayland has the concept of "inert resources". For instance when a surface which also happens to be a subsurface is destroyed by the client, the subsurface becomes inert.

Being inert means that all requests except the destructor are ignored. Being inert also means being a pain in the ass.

So this needs to be done for resources that can become inert:

1. When the resource becomes inert, destroy our wlr_* struct (removes listeners, releases memory). Set the resource user_data to NULL. Do not destroy the resource.
2. For each request made to a resource that can be inert: add a NULL check, ignore the request if the resource is inert.
3. When the client calls the destructor request on the resource: wl_resource_destroy(resource).
4. When the resource is destroyed, if the resource isn't inert (user_data NULL check), destroy our wlr_* struct.

[emersion]

I forgot to mention that inert semantics can be even more annoying, for instance see the last comment of this patch about wl_seat: https://patchwork.freedesktop.org/patch/43356/