[PATCH v2] server: add wl_display_destroy_clients()

[Simon Ser]

>-------- Original Message --------
>Subject: Re: [PATCH v2] server: add wl_display_destroy_clients()
>Local Time: December 13, 2017 1:39 PM
>UTC Time: December 13, 2017 12:39 PM
>From: ppaalanen at gmail.com
>To: Simon Ser <contact at emersion.fr>
>wayland-devel at lists.freedesktop.org <wayland-devel at lists.freedesktop.org>
>
>On Wed, 13 Dec 2017 06:59:10 -0500
>Simon Ser <contact at emersion.fr> wrote:
>
>> >-------- Original Message --------
>> >Subject: Re: [PATCH v2] server: add wl_display_destroy_clients()
>> >Local Time: December 13, 2017 12:11 PM
>> >UTC Time: December 13, 2017 11:11 AM
>> >From: ppaalanen at gmail.com
>> >To: emersion <contact at emersion.fr>
>> >wayland-devel at lists.freedesktop.org
>> >
>> >On Wed, 13 Dec 2017 11:51:19 +0100
>> > emersion contact at emersion.fr wrote:  
>> >>Bug [1] reported that wl_display_destroy() doesn't destroy clients, so
>> >> client socket file descriptors are being kept open until the compositor
>> >> process exits.
>> >>Patch [2] proposed to destroy clients in wl_display_destroy(). The
>> >> patch was not accepted because doing so changes the ABI.
>> >>Thus, a new wl_display_destroy_clients() function is added in this
>> >> patch. It should be called by compositors right before
>> >> wl_display_destroy().
>> >>[1] https://bugs.freedesktop.org/show_bug.cgi?id=99142
>> >> [2] https://patchwork.freedesktop.org/patch/128832/
>> >>Signed-off-by: emersion contact at emersion.fr
>
>Hi,
>
>ah, your real name is Simon Ser? We need it in the Signed-off-by tag
>for the tag to be valid, do you mind if I fix it while eventually landing
>this patch?
>
>The Linux kernel patch submission guidelines say it's required, nick
>names won't do.

Yes, my real name is Simon Ser. Sorry, I wasn't aware of this rule. I'm fine
with you fixing it.

>> >
>> > Hi,
>> >
>> > very good!
>> >
>> > Reviewed-by: Pekka Paalanen pekka.paalanen at collabora.co.uk  
>> 
>> Thanks for your review!
>> 
>> > Do you have your compositor now calling this without issues?  
>> 
>> Yes! I've tested the new function and it works properly. Here's the patch:
>> https://github.com/emersion/wlroots/commit/71d344cc616021a1d1ed6e075f8c7e0d15b0deb0
>
>Cool.
>
>> > It would be awesome to have a test in the libwayland test suite calling
>> > this function and checking a client gets destroyed, especially as we
>> > don't have Weston using this yet.  
>> 
>> I'll try to make another patch for this.
>
>Appreciated.
>
>
>Thanks,
>pq