xcb: Remove free XID cache

cairo-xcb kept a cache of free xid to avoid calling xcb_generate_id()
later. However, this is unsafe: When libxcb runs out of ids, it asks the
X11 server for an empty range of ids to use. The X11 server of course
does not know about cairo's cache and could hand out an id that cairo
will use again later. This would then result in BadIdChoice errors
later.

Fix this by simply removing the whole cache.

Fixes: https://gitlab.freedesktop.org/cairo/cairo/-/issues/434
Signed-off-by: Uli Schlachter <psychon@znc.in>
This commit is contained in:
Uli Schlachter 2021-05-02 17:49:32 +02:00
parent 4f61b765c9
commit c16094fc44
6 changed files with 8 additions and 69 deletions

View file

@ -42,7 +42,7 @@ _cairo_xcb_connection_create_pixmap (cairo_xcb_connection_t *connection,
uint16_t width,
uint16_t height)
{
xcb_pixmap_t pixmap = _cairo_xcb_connection_get_xid (connection);
xcb_pixmap_t pixmap = xcb_generate_id (connection->xcb_connection);
assert (width > 0);
assert (height > 0);
@ -57,7 +57,6 @@ _cairo_xcb_connection_free_pixmap (cairo_xcb_connection_t *connection,
xcb_pixmap_t pixmap)
{
xcb_free_pixmap (connection->xcb_connection, pixmap);
_cairo_xcb_connection_put_xid (connection, pixmap);
}
xcb_gcontext_t
@ -66,7 +65,7 @@ _cairo_xcb_connection_create_gc (cairo_xcb_connection_t *connection,
uint32_t value_mask,
uint32_t *values)
{
xcb_gcontext_t gc = _cairo_xcb_connection_get_xid (connection);
xcb_gcontext_t gc = xcb_generate_id (connection->xcb_connection);
xcb_create_gc (connection->xcb_connection, gc, drawable,
value_mask, values);
return gc;
@ -77,7 +76,6 @@ _cairo_xcb_connection_free_gc (cairo_xcb_connection_t *connection,
xcb_gcontext_t gc)
{
xcb_free_gc (connection->xcb_connection, gc);
_cairo_xcb_connection_put_xid (connection, gc);
}
void

View file

@ -79,7 +79,6 @@ _cairo_xcb_connection_render_free_picture (cairo_xcb_connection_t *connection,
{
assert (connection->flags & CAIRO_XCB_HAS_RENDER);
xcb_render_free_picture (connection->xcb_connection, picture);
_cairo_xcb_connection_put_xid (connection, picture);
}
void
@ -133,7 +132,6 @@ _cairo_xcb_connection_render_free_glyph_set (cairo_xcb_connection_t *connec
{
assert (connection->flags & CAIRO_XCB_HAS_RENDER);
xcb_render_free_glyph_set (connection->xcb_connection, glyphset);
_cairo_xcb_connection_put_xid (connection, glyphset);
}
void

View file

@ -43,7 +43,7 @@ _cairo_xcb_connection_shm_attach (cairo_xcb_connection_t *connection,
uint32_t id,
cairo_bool_t readonly)
{
uint32_t segment = _cairo_xcb_connection_get_xid (connection);
uint32_t segment = xcb_generate_id (connection->xcb_connection);
assert (connection->flags & CAIRO_XCB_HAS_SHM);
xcb_shm_attach (connection->xcb_connection, segment, id, readonly);
return segment;
@ -109,7 +109,6 @@ _cairo_xcb_connection_shm_detach (cairo_xcb_connection_t *connection,
{
assert (connection->flags & CAIRO_XCB_HAS_SHM);
xcb_shm_detach (connection->xcb_connection, segment);
_cairo_xcb_connection_put_xid (connection, segment);
}
#endif /* CAIRO_HAS_XCB_SHM_FUNCTIONS */

View file

@ -255,7 +255,7 @@ pixmap_depths_usable (cairo_xcb_connection_t *connection,
cairo_bool_t success = TRUE;
int depth, i, j;
pixmap = _cairo_xcb_connection_get_xid (connection);
pixmap = xcb_generate_id (connection->xcb_connection);
for (depth = 1, i = 0; depth <= 32; depth++) {
if (missing & DEPTH_MASK(depth)) {
@ -275,8 +275,6 @@ pixmap_depths_usable (cairo_xcb_connection_t *connection,
free (create_error);
}
_cairo_xcb_connection_put_xid (connection, pixmap);
return success;
}
@ -462,10 +460,9 @@ can_use_shm (cairo_xcb_connection_t *connection)
return FALSE;
}
shmseg = _cairo_xcb_connection_get_xid (connection);
shmseg = xcb_generate_id (connection->xcb_connection);
cookie[0] = xcb_shm_attach_checked (c, shmseg, shmid, FALSE);
cookie[1] = xcb_shm_detach_checked (c, shmseg);
_cairo_xcb_connection_put_xid (connection, shmseg);
error = xcb_request_check (c, cookie[0]);
if (error != NULL)
@ -586,8 +583,6 @@ _device_destroy (void *device)
#endif
_cairo_freepool_fini (&connection->shm_info_freelist);
_cairo_freepool_fini (&connection->xid_pool);
CAIRO_MUTEX_FINI (connection->shm_mutex);
CAIRO_MUTEX_FINI (connection->screens_mutex);
@ -662,10 +657,6 @@ _cairo_xcb_connection_get (xcb_connection_t *xcb_connection)
goto unlock;
}
cairo_list_init (&connection->free_xids);
_cairo_freepool_init (&connection->xid_pool,
sizeof (cairo_xcb_xid_t));
cairo_list_init (&connection->shm_pools);
cairo_list_init (&connection->shm_pending);
_cairo_freepool_init (&connection->shm_info_freelist,
@ -766,43 +757,6 @@ _cairo_xcb_connection_get_xrender_format_for_visual (cairo_xcb_connection_t *con
return format ? format->xrender_format : XCB_NONE;
}
void
_cairo_xcb_connection_put_xid (cairo_xcb_connection_t *connection,
uint32_t xid)
{
cairo_xcb_xid_t *cache;
assert (CAIRO_MUTEX_IS_LOCKED (connection->device.mutex));
cache = _cairo_freepool_alloc (&connection->xid_pool);
if (likely (cache != NULL)) {
cache->xid = xid;
cairo_list_add (&cache->link, &connection->free_xids);
}
}
uint32_t
_cairo_xcb_connection_get_xid (cairo_xcb_connection_t *connection)
{
uint32_t xid;
assert (CAIRO_MUTEX_IS_LOCKED (connection->device.mutex));
if (! cairo_list_is_empty (&connection->free_xids)) {
cairo_xcb_xid_t *cache;
cache = cairo_list_first_entry (&connection->free_xids,
cairo_xcb_xid_t,
link);
xid = cache->xid;
cairo_list_del (&cache->link);
_cairo_freepool_free (&connection->xid_pool, cache);
} else {
xid = xcb_generate_id (connection->xcb_connection);
}
return xid;
}
/**
* cairo_xcb_device_get_connection:
* @device: a #cairo_device_t for the XCB backend

View file

@ -226,9 +226,6 @@ struct _cairo_xcb_connection {
const xcb_query_extension_reply_t *shm;
xcb_render_sub_pixel_t *subpixel_orders;
cairo_list_t free_xids;
cairo_freepool_t xid_pool;
cairo_mutex_t shm_mutex;
cairo_list_t shm_pools;
cairo_list_t shm_pending;
@ -323,13 +320,6 @@ _cairo_xcb_connection_acquire (cairo_xcb_connection_t *connection)
return cairo_device_acquire (&connection->device);
}
cairo_private uint32_t
_cairo_xcb_connection_get_xid (cairo_xcb_connection_t *connection);
cairo_private void
_cairo_xcb_connection_put_xid (cairo_xcb_connection_t *connection,
uint32_t xid);
static inline void
_cairo_xcb_connection_release (cairo_xcb_connection_t *connection)
{

View file

@ -128,7 +128,7 @@ _cairo_xcb_picture_create (cairo_xcb_screen_t *screen,
cairo_list_add (&surface->link, &screen->pictures);
surface->screen = screen;
surface->picture = _cairo_xcb_connection_get_xid (screen->connection);
surface->picture = xcb_generate_id (screen->connection->xcb_connection);
surface->pixman_format = pixman_format;
surface->xrender_format = xrender_format;
@ -308,7 +308,7 @@ _cairo_xcb_surface_ensure_picture (cairo_xcb_surface_t *surface)
values[0] = surface->precision;
}
surface->picture = _cairo_xcb_connection_get_xid (surface->connection);
surface->picture = xcb_generate_id (surface->connection->xcb_connection);
_cairo_xcb_connection_render_create_picture (surface->connection,
surface->picture,
surface->drawable,
@ -4232,7 +4232,7 @@ _cairo_xcb_scaled_font_get_glyphset_info_for_format (cairo_xcb_connection_t *c,
info = &priv->glyphset_info[glyphset_index];
if (info->glyphset == XCB_NONE) {
info->glyphset = _cairo_xcb_connection_get_xid (c);
info->glyphset = xcb_generate_id (c->xcb_connection);
info->xrender_format = c->standard_formats[info->format];
_cairo_xcb_connection_render_create_glyph_set (c,