[PATCH] xwayland: Close the shm fd as early as possible

Daniel Stone daniel at fooishbar.org
Wed Sep 28 15:25:51 UTC 2016


On 5 February 2016 at 14:25, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Fri,  5 Feb 2016 14:44:29 +0100
> Rui Matos <tiagomatos at gmail.com> wrote:
>
>> Keeping the shm fd open beyond pixmap creation means we can easily
>> reach the open file descriptor limit if an X client asks us to create
>> that many pixmaps. Instead, let's get the wl_buffer immediatly so that
>> we can destroy the shm pool and close the fd before being asked to
>> create more.
>> ---
>>
>> On Fri, Feb 5, 2016 at 1:53 PM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>> > You don't need the fd or the pool to write into the buffer, or attach
>> > and commit the wl_buffer, or...
>>
>> You are right, of course. My initial attempt at this was crashing and
>> I quickly dismissed this approach without investigating why it was
>> crashing more closely. And sure enough, it was just a thinko on my
>> part.
>
> the idea here looks fine to me.
>
> However, I've been wondering, surely there was a reason why it wasn't
> coded like this in the first place?
>
> Could you check if this patch causes Xwayland to create lots of
> wl_buffers it will never attach and commit to a wl_surface? If it does,
> that was probably the reason.
>
> If that is true, then one has to weigh between creating wl_buffers
> immediately vs. creating them only when actually needed for a
> wl_surface.
>
> Maybe Daniel knows? CC'ing also Olivier for his information.

No good reason, in the cold hard light of day. Mostly just code
evolution, where in previous iterations of Xwayland this was called
for pretty much all backing storage, so we wanted to avoid creating
unnecessary server-side resources, now we'll pretty much always create
a server-side resource when we land here.

Ship it!

Reviewed-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel


More information about the xorg-devel mailing list