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

[Olivier Fourdan]

> > > [...]
> > > 
> > > 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.
> > 
> > Was that evaluated?

I guess the risk here is that we might end up creating useless wl_buffers.

Still, the benefit of closing the fd as soon as possible is greater than the risk of creating too many unused wl_buffer, so I think it's worth it.

(Not convinced? try running 10 instances of "gtk3-demo --run=cursors" in Wayland)

> > AFAICS, when using glamor, the only code path that still uses the
> > xwl_shm_*_pixmap() is the cursor code, so even with glamor, running several
> > X11 apps which create several cursors each will quickly cause Xwayland to
> > reach the limit of file descriptors.
> >  
> > > 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.
> > 
> > I cannot tell why this was done the way it is, but I think this patch does
> > improve things for e.g. downstream bug
> > https://bugzilla.redhat.com/show_bug.cgi?id=1373451

That particular bug is possibly a leak of cursors in Qt, but still, there is a weakness in Xwayland.

so, weighing the pros and cons of this patch, FWIW:

Reviewed-by: Olivier Fourdan <ofourdan at redhat.com>

Unless someone else is opposed to this patch, I'm in...

Cheers,
Olivier