Attachment 139297 Details for Bug 98883 – Handle-XShmAttach-failures-v1.patch

[patch] Handle-XShmAttach-failures-v1.patch

Handle-XShmAttach-failures-v1.patch (text/plain), 2.63 KB, created by Daniel van Vugt on 2018-05-03 10:27:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel van Vugt

Created: 2018-05-03 10:27:18 UTC

Size: 2.63 KB

patch

obsolete

>From 4f4d3b7d725ed48446306eb459e3a31a467a732d Mon Sep 17 00:00:00 2001
>From: Daniel van Vugt <daniel.van.vugt@canonical.com>
>Date: Thu, 3 May 2018 18:08:31 +0800
>Subject: [PATCH] Handle XShmAttach failures.
>
>XShmAttach returns True, even on error. And when an error occurs it
>happens asynchronously, after we had thought that shm was working. The
>result would be at best no image on screen if the error was caught later
>by the client, or worse a crash if the client didn't handle the X error.
>Either way, returning a successful result from _cairo_xlib_shm_pool_create
>which may actually have failed is semantically incorrect. So detect when
>XShmAttach has failed and handle it gracefully. The user will never know
>that it failed because a fallback already exists.
>
>https://launchpad.net/bugs/1751252
>https://bugs.freedesktop.org/show_bug.cgi?id=98883
>---
> src/cairo-xlib-surface-shm.c | 28 ++++++++++++++++++++++++----
> 1 file changed, 24 insertions(+), 4 deletions(-)
>
>diff --git a/src/cairo-xlib-surface-shm.c b/src/cairo-xlib-surface-shm.c
>index a339d0c64..2f638bf21 100644
>--- a/src/cairo-xlib-surface-shm.c
>+++ b/src/cairo-xlib-surface-shm.c
>@@ -571,7 +571,8 @@ _cairo_xlib_shm_pool_create(cairo_xlib_display_t *display,
>     Display *dpy = display->display;
>     cairo_xlib_shm_t *pool;
>     size_t bytes, maxbits = 16, minbits = MIN_BITS;
>-    Status success;
>+    Bool success;
>+    int (*old_handler) (Display *display, XErrorEvent *event);
> 
>     pool = malloc (sizeof (cairo_xlib_shm_t));
>     if (pool == NULL)
>@@ -599,11 +600,30 @@ _cairo_xlib_shm_pool_create(cairo_xlib_display_t *display,
> 	goto cleanup;
>     }
> 
>+    CAIRO_MUTEX_LOCK (_cairo_xlib_display_mutex);
>+    XLockDisplay (dpy);
>+    XSync (dpy, False);
>+    old_handler = XSetErrorHandler (_check_error_handler);
>+
>     pool->attached = XNextRequest (dpy);
>+    _x_error_occurred = FALSE;
>     success = XShmAttach (dpy, &pool->shm);
>-#if !IPC_RMID_DEFERRED_RELEASE
>-    XSync (dpy, FALSE);
>-#endif
>+
>+    /* XShmAttach is hard-coded in libXext to return True, even on failure.
>+     * So we need to try harder to detect failure and then return NULL instead
>+     * of crashing from a delayed X error. Returning NULL will at least trigger
>+     * the fallback to non-shm and the user will never notice. This is all
>+     * unfortunate for performance, but better than crashing or failing to get
>+     * an image on screen.
>+     */
>+    XSync (dpy, False);
>+    if (_x_error_occurred)
>+        success = False;
>+
>+    XSetErrorHandler (old_handler);
>+    XUnlockDisplay (dpy);
>+    CAIRO_MUTEX_UNLOCK (_cairo_xlib_display_mutex);
>+
>     shmctl (pool->shm.shmid, IPC_RMID, NULL);
> 
>     if (! success)
>-- 
>2.17.0
>

Actions: View | Splinter Review

Attachments on bug 98883: 139297

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.