[RFC PATCH xserver v2] xwayland: Monitor client states to destroy callbacks

[Michel Dänzer]

On 07/03/17 08:08 PM, Olivier Fourdan wrote:
> Client resources can survive the client itself, in which case we
> may end up in our sync callback trying to access client's data after
> it's been freed/reclaimed.
> 
> Add a ClientStateCallback handler to monitor the client state changes
> and clear the sync callback set up by the glamor drm code if any.
> 
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  v2: [facepalm] forgot to use the state->callback
> 
>  hw/xwayland/xwayland-glamor.c | 52 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
> index 65c3c00..9c4297e 100644
> --- a/hw/xwayland/xwayland-glamor.c
> +++ b/hw/xwayland/xwayland-glamor.c
> @@ -38,6 +38,9 @@
>  #include <dri3.h>
>  #include "drm-client-protocol.h"
>  
> +static DevPrivateKeyRec xwlGlamorPrivateKeyRec;
> +#define xwlGlamorPrivateKey (&xwlGlamorPrivateKeyRec)

"xwlGlamorPrivateKey" seems a bit generic — this key can't be used for
non-client privates at the same time, can it?


> @@ -441,8 +473,7 @@ sync_callback(void *data, struct wl_callback *callback, uint32_t serial)
>          dri3_send_open_reply(client, state->fd);
>          AttendClient(client);
>      }

sync_callback should no longer need to check client->clientGone.


> -    free(state);
> -    wl_callback_destroy(callback);
> +    free_xwl_auth_state (client, state);
>  }

Space between function name and opening parens doesn't match coding
style, though there does seem to be some precedent in hw/xwayland.
(There's another instance of this in the patch)


Other than these, looks good to me.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer