[PULL/RFC] compositor-x11 destructors v1

[Kristian Høgsberg]

On Tue, Jan 3, 2012 at 9:53 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> Hi,
>
> here is the majority of the compositor cleanups I did for
> compositor-x11. They depend on the patch "server: add
> wl_input_device_fini()" to the Wayland core.

Thanks, that's a great cleanup series.  Pulled and pushed.

> Most of it is trivial, but destroying global objects (input device,
> maybe outputs) have their caveats. The main thing still not destroyed
> after this patch set are the wl_client structures for clients that
> exist at the time the compositor shuts down. I tried to destroy all
> clients within wl_display_destroy(), but that does not work together
> with destroying the global objects at backend shutdown time. The
> client structures will hold references to objects the backend has
> destroyed, so destroying the client hits use-after-free.

We could add a wl_display function to disconnect all clients, but it's
a problem we need to solve regardless for hotplug.  I think that it
may be better to just disconnect all clients before we start tearing
down global objects.

> I guess solving that properly requires a proper implementation of
> global object destructors, that not only announces the globals going
> away to the clients, but also takes care of the corresponding
> wl_resource objects, and the possible races between sending a destroy
> event and a client processing it.
>
> If you want, you can drop the compositor-x11 patches for now, they are
> the ones that (are supposed to) destroy globals. In my tests, they did
> not cause any harm, though, as we do not destroy the remaining
> wl_clients.
>
> Do you have a plan on how to handle removing global objects dynamically,
> e.g. an input device?

We have the global_remove event, that we use to signal that a global
has been removed.  Clients are exected to destroy the corresponding
local object in response to that, but as you say, there are a few
races around that that we need to handle correctly:

1) Bind request for a global object that has just been removed.  This
could happen if a client sends a bind request just as the server
destroys the object, for example.  We don't want this to be an error
condition, as that might crash correct clients.  Instead we want it to
be a no-op and eventually the client will receive the global_remove
event, and destroy the client object.  I think we can insert a zombie
object server side that just ignores all requests and then wait for
the client to destroy that.

2) Client sending requests to a bound global object after the global
object has been destroyed.  This is similar to 1), in that we can just
insert a zombie object when we destroy the object and then wait for
clients to destroy the object.

Kristian