Subject: Re: bug#63589: [PATCH] 29.0.91; crash after creating graphical frames via emacsclient when compiled with cairo-xcb

From: Thiago Melo <[email-masked]>
Date: Wed, 24 May 2023 11:54:30 +0000

> What triggers the bug with Lucid and GTK builds? the patch you
> proposed or calls to delete-terminal?  If the latter, then this is a
> separate issue, and at least the Cairo-xcb build will behave like the
> other builds in that scenario.  Right?

Sorry for not being clear. I meant calls to `delete-terminal', when
Emacs is built with Cairo-XCB, and regardless of the toolkit. It's a
similar scenario to trigger the bug: launch Emacs daemon, open one or
more graphical frames, call the elisp function`delete-terminal' (all
graphical frames and the display are closed because of it), open a new
graphical frame...

My last patch doesn't fix it because it only works when
`(delete-frame)' is called instead. This situation is specific for
toolkitless + Cairo-XCB Emacs, since here it ends up calling
`x_delete_terminal' -> `XCloseDisplay'. Other toolkits don't call
`x_delete_terminal' here, due to the logic at `delete_frame'.

I hope I was more clear this time.

My opinion is that it's all the same issue, which boils down to
Cairo-XCB requiring more manual memory management than Cairo-XLib by
design or limitation. I think we should really consider the approach
from my first patch, which is ensuring that the Cairo-XCB device is
cleaned up before calling XCloseDisplay. Everything else is a
workaround. If the patch is considered too unsafe or too big, then we
must clarify the specifics of what makes it so, so the matter can be
addressed in a better way.

Here's another patch, similar to the first one, but it only acts at
`x_delete_terminal', and without storing global references. The
strategy is similar to the one used at
`ftcrfont_get_default_font_options'. It creates a dummy pixmap, then a
dummy cairo xcb surface from it, then it extracts the cairo device
from the surface, and then cleans up them all.

#+begin_src diff
--- a/src/xterm.c    2023-05-24 12:42:14.873824624 +0200
+++ b/src/xterm.c    2023-05-24 13:45:23.798382193 +0200
@@ -30841,6 +30841,30 @@
      closing all the displays.  */
       XrmDestroyDatabase (dpyinfo->rdb);
 #endif
+#ifdef USE_CAIRO_XCB_SURFACE
+      /* Ensure that the cairo device is destroyed before closing
+         connection (Bug#63589).  For that, we create a drawable, an XCB
+         surface for that drawable, and then we get the device reference
+         from there.  */
+
+      Pixmap drawable;
+      cairo_surface_t *surface;
+
+      drawable = XCreatePixmap (dpyinfo->display, dpyinfo->root_window,
+                1, 1, dpyinfo->n_planes);
+      surface = cairo_xcb_surface_create (dpyinfo->xcb_connection, drawable,
+                      dpyinfo->xcb_visual, 1, 1);
+
+      if (cairo_surface_status (surface) == CAIRO_STATUS_SUCCESS)
+    {
+      cairo_device_t *cairo_device;
+      cairo_device = cairo_device_reference (cairo_surface_get_device
(surface));
+      cairo_surface_destroy (surface);
+      cairo_device_finish (cairo_device);
+      cairo_device_destroy (cairo_device);
+    }
+      XFreePixmap (dpyinfo->display, drawable);
+#endif
 #ifdef USE_GTK
       xg_display_close (dpyinfo->display);
 #else
#+end_src