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

[Rui Matos]

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.

Thanks!

Rui

 hw/xwayland/xwayland-shm.c | 46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/hw/xwayland/xwayland-shm.c b/hw/xwayland/xwayland-shm.c
index 7072be4..25ee03c 100644
--- a/hw/xwayland/xwayland-shm.c
+++ b/hw/xwayland/xwayland-shm.c
@@ -36,7 +36,6 @@
 
 struct xwl_pixmap {
     struct wl_buffer *buffer;
-    int fd;
     void *data;
     size_t size;
 };
@@ -174,9 +173,13 @@ PixmapPtr
 xwl_shm_create_pixmap(ScreenPtr screen,
                       int width, int height, int depth, unsigned int hint)
 {
-    PixmapPtr pixmap;
+    struct xwl_screen *xwl_screen = xwl_screen_get(screen);
     struct xwl_pixmap *xwl_pixmap;
+    struct wl_shm_pool *pool;
+    PixmapPtr pixmap;
     size_t size, stride;
+    uint32_t format;
+    int fd;
 
     if (hint == CREATE_PIXMAP_USAGE_GLYPH_PICTURE ||
         (width == 0 && height == 0) || depth < 15)
@@ -194,12 +197,12 @@ xwl_shm_create_pixmap(ScreenPtr screen,
     size = stride * height;
     xwl_pixmap->buffer = NULL;
     xwl_pixmap->size = size;
-    xwl_pixmap->fd = os_create_anonymous_file(size);
-    if (xwl_pixmap->fd < 0)
+    fd = os_create_anonymous_file(size);
+    if (fd < 0)
         goto err_free_xwl_pixmap;
 
     xwl_pixmap->data = mmap(NULL, size, PROT_READ | PROT_WRITE,
-                                  MAP_SHARED, xwl_pixmap->fd, 0);
+                                  MAP_SHARED, fd, 0);
     if (xwl_pixmap->data == MAP_FAILED)
         goto err_close_fd;
 
@@ -208,6 +211,15 @@ xwl_shm_create_pixmap(ScreenPtr screen,
                                         stride, xwl_pixmap->data))
         goto err_munmap;
 
+    format = shm_format_for_depth(pixmap->drawable.depth);
+    pool = wl_shm_create_pool(xwl_screen->shm, fd, xwl_pixmap->size);
+    xwl_pixmap->buffer = wl_shm_pool_create_buffer(pool, 0,
+                                                   pixmap->drawable.width,
+                                                   pixmap->drawable.height,
+                                                   pixmap->devKind, format);
+    wl_shm_pool_destroy(pool);
+    close(fd);
+
     xwl_pixmap_set_private(pixmap, xwl_pixmap);
 
     return pixmap;
@@ -215,7 +227,7 @@ xwl_shm_create_pixmap(ScreenPtr screen,
  err_munmap:
     munmap(xwl_pixmap->data, size);
  err_close_fd:
-    close(xwl_pixmap->fd);
+    close(fd);
  err_free_xwl_pixmap:
     free(xwl_pixmap);
  err_destroy_pixmap:
@@ -233,7 +245,6 @@ xwl_shm_destroy_pixmap(PixmapPtr pixmap)
         if (xwl_pixmap->buffer)
             wl_buffer_destroy(xwl_pixmap->buffer);
         munmap(xwl_pixmap->data, xwl_pixmap->size);
-        close(xwl_pixmap->fd);
         free(xwl_pixmap);
     }
 
@@ -243,26 +254,7 @@ xwl_shm_destroy_pixmap(PixmapPtr pixmap)
 struct wl_buffer *
 xwl_shm_pixmap_get_wl_buffer(PixmapPtr pixmap)
 {
-    struct xwl_screen *xwl_screen = xwl_screen_get(pixmap->drawable.pScreen);
-    struct xwl_pixmap *xwl_pixmap = xwl_pixmap_get(pixmap);
-    struct wl_shm_pool *pool;
-    uint32_t format;
-
-    if (xwl_pixmap->buffer)
-        return xwl_pixmap->buffer;
-
-    pool = wl_shm_create_pool(xwl_screen->shm,
-                              xwl_pixmap->fd, xwl_pixmap->size);
-
-    format = shm_format_for_depth(pixmap->drawable.depth);
-    xwl_pixmap->buffer = wl_shm_pool_create_buffer(pool, 0,
-                                                   pixmap->drawable.width,
-                                                   pixmap->drawable.height,
-                                                   pixmap->devKind, format);
-
-    wl_shm_pool_destroy(pool);
-
-    return xwl_pixmap->buffer;
+    return xwl_pixmap_get(pixmap)->buffer;
 }
 
 Bool
-- 
2.5.0