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

[Thiago Melo <tmdmelo@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]

On Thu, May 25, 2023 at 10:33 AM Po Lu <luangruo@yahoo.com> wrote:
> Did you build with checking?

Yes. Here are the configure options I've been using to test it:

./configure --without-all --with-x-toolkit=no
--without-compress-install --without-tree-sitter --without-json
--with-cairo --enable-checking='yes,glyphs'
--enable-check-lisp-object-type  CFLAGS='-O0 -g3'

Let me know if there are relevant settings differences.

> Because when I last tried, this assert
> triggered with the second frame created.

I'm not sure if you tested the last patch I sent as it is, or if you
previously did assert tests on your own with the device returned by
`cairo_xcb_surface_create' at `x_begin_cr_clip'. Assuming it's the
latter, then please pay close attention at this change I made to the
code:


    if (cairo_surface_status (surface) == CAIRO_STATUS_SUCCESS)
      eassert (FRAME_DISPLAY_INFO (f)->cairo_device
           == cairo_surface_get_device (surface));


Notice that, before I do the assert, I first check if the surface
returned by `cairo_xcb_surface_create' is good. One thing that I
observed when debugging is that, every time a new frame is created,
this part of the code is hit 3 times. The first time, the surface it
returns is always a bad one, which might even have some random garbage
value for the device. The other 2 times, it's a proper xcb surface,
and they always have the same device in common. Same thing with
subsequent calls, it's always the same cairo-xcb device. Similar
behavior when I make tooltips appear. I've attached a gdb session log
showing it.

> If we are going down this route, I think we should save each distinct
> device returned by `cairo_surface_get_device', and delete each of them
> upon the terminal being deleted.
> As I explained, I saw that function return different devices for the
> same XCB connection, which is definitely a problem with Cairo.

Considering my observation above, it doesn't seem that different
_valid_ devices are being created. But if that is really the case and
I'm missing it, then yes, we make a dynamic list of devices instead.
Consider also that, with the changes I proposed, a device reference is
kept from the beginning, which might influence the results here.

[-- Attachment #2: gdb-session--cairo-xcb-device.org --]
[-- Type: application/vnd.lotus-organizer, Size: 5537 bytes --]