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

[Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

Thiago Melo <tmdmelo@gmail.com> writes:

> 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.

What version of Cairo did you test?

>> 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.

Or let's just disable this by default, which is really the better
solution until some people get their act together and fix this
misdesign.