[cairo-commit] 3 commits - src/cairo-xcb-connection.c src/cairo-xcb-connection-shm.c src/cairo-xcb-private.h src/cairo-xcb-shm.c src/cairo-xcb-surface.c src/cairo-xcb-surface-core.c src/cairo-xcb-surface-render.c
M. Joonas Pihlaja
joonas at kemper.freedesktop.org
Sun Jan 2 13:51:34 PST 2011
src/cairo-xcb-connection-shm.c | 10 +--
src/cairo-xcb-connection.c | 11 +++
src/cairo-xcb-private.h | 9 ++-
src/cairo-xcb-shm.c | 113 ++++++++++++++++++++++++++++++++---------
src/cairo-xcb-surface-core.c | 19 +++---
src/cairo-xcb-surface-render.c | 19 +++---
src/cairo-xcb-surface.c | 21 +++----
7 files changed, 140 insertions(+), 62 deletions(-)
New commits:
commit e5f54bb9f34a463cd10240451dd5d29a735c4506
Author: Uli Schlachter <psychon at znc.in>
Date: Sat Dec 25 14:46:43 2010 +0100
XCB: Make sure SHM memory isn't reused too early
This commit delays the return of a SHM area to the free pool. When
_cairo_xcb_shm_info_destroy is called, it now adds the cairo_xcb_shm_info_t to a
list of pending memory areas and sends a GetInputFocus request to the server.
This cairo_xcb_shm_info_t is only really freed when the GetInputFocus request
completes. To avoid unnecessarily waiting for the X server, we check via
xcb_poll_for_reply for the reply which returns immediately if the reply isn't
received yet.
This commits fixes a race where the shared memory area is reused before the X
server finished reading data from it. This does NOT fix races where cairo draws
something new to the same cairo_xcb_shm_info_t while the X server still reads
from it. However, there doesn't seem to exist any code currently where the shm
info isn't immediately destroyed after it was used.
This commit fixes the following tests for xcb-render-0.0 if SHM is enabled:
joins mask mask-transformed-image push-group push-group-color radial-gradient
radil-gradient-mask radial-gradient-mask-source radial-gradient-one-stop
radial-gradient-source smask smask-mask smask-paint
This also fixes mesh-pattern-transformed for all the xcb boilerplate "backends".
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-xcb-connection.c b/src/cairo-xcb-connection.c
index 74b9f21..73f70b1 100644
--- a/src/cairo-xcb-connection.c
+++ b/src/cairo-xcb-connection.c
@@ -479,6 +479,10 @@ _device_flush (void *device)
if (unlikely (status))
return status;
+#if CAIRO_HAS_XCB_SHM_FUNCTIONS
+ _cairo_xcb_connection_shm_mem_pools_flush (connection);
+#endif
+
CAIRO_MUTEX_LOCK (connection->screens_mutex);
cairo_list_foreach_entry (screen, cairo_xcb_screen_t,
&connection->screens, link)
@@ -540,6 +544,12 @@ _device_finish (void *device)
_cairo_xcb_screen_finish (screen);
}
+#if CAIRO_HAS_XCB_SHM_FUNCTIONS
+ /* _cairo_xcb_screen_finish finishes surfaces. If any of those surfaces had
+ * a fallback image, we might have done a SHM PutImage. */
+ _cairo_xcb_connection_shm_mem_pools_flush (connection);
+#endif
+
if (was_cached)
cairo_device_destroy (device);
}
@@ -643,6 +653,7 @@ _cairo_xcb_connection_get (xcb_connection_t *xcb_connection)
sizeof (cairo_xcb_xid_t));
cairo_list_init (&connection->shm_pools);
+ cairo_list_init (&connection->shm_pending);
_cairo_freepool_init (&connection->shm_info_freelist,
sizeof (cairo_xcb_shm_info_t));
diff --git a/src/cairo-xcb-private.h b/src/cairo-xcb-private.h
index 7da888c..b6b25ce 100644
--- a/src/cairo-xcb-private.h
+++ b/src/cairo-xcb-private.h
@@ -68,6 +68,8 @@ struct _cairo_xcb_shm_info {
uint32_t offset;
void *mem;
cairo_xcb_shm_mem_pool_t *pool;
+ xcb_get_input_focus_cookie_t sync;
+ cairo_list_t pending;
};
struct _cairo_xcb_surface {
@@ -187,6 +189,7 @@ struct _cairo_xcb_connection {
cairo_mutex_t shm_mutex;
cairo_list_t shm_pools;
+ cairo_list_t shm_pending;
cairo_freepool_t shm_info_freelist;
cairo_mutex_t screens_mutex;
@@ -271,6 +274,9 @@ cairo_private void
_cairo_xcb_shm_info_destroy (cairo_xcb_shm_info_t *shm_info);
cairo_private void
+_cairo_xcb_connection_shm_mem_pools_flush (cairo_xcb_connection_t *connection);
+
+cairo_private void
_cairo_xcb_connection_shm_mem_pools_fini (cairo_xcb_connection_t *connection);
cairo_private void
diff --git a/src/cairo-xcb-shm.c b/src/cairo-xcb-shm.c
index fae020a..c3e3d35 100644
--- a/src/cairo-xcb-shm.c
+++ b/src/cairo-xcb-shm.c
@@ -49,6 +49,11 @@
typedef struct _cairo_xcb_shm_mem_block cairo_xcb_shm_mem_block_t;
+typedef enum {
+ PENDING_WAIT,
+ PENDING_POLL
+} shm_wait_type_t;
+
struct _cairo_xcb_shm_mem_block {
unsigned int bits;
cairo_list_t link;
@@ -412,12 +417,12 @@ _cairo_xcb_shm_mem_pool_destroy (cairo_xcb_shm_mem_pool_t *pool)
free (pool);
}
-void
-_cairo_xcb_shm_info_destroy (cairo_xcb_shm_info_t *shm_info)
+static void
+_cairo_xcb_shm_info_finalize (cairo_xcb_shm_info_t *shm_info)
{
cairo_xcb_connection_t *connection = shm_info->connection;
- CAIRO_MUTEX_LOCK (connection->shm_mutex);
+ assert (CAIRO_MUTEX_IS_LOCKED (connection->shm_mutex));
_cairo_xcb_shm_mem_pool_free (shm_info->pool, shm_info->mem);
_cairo_freepool_free (&connection->shm_info_freelist, shm_info);
@@ -442,8 +447,38 @@ _cairo_xcb_shm_info_destroy (cairo_xcb_shm_info_t *shm_info)
cairo_list_move (head.next, &connection->shm_pools);
}
+}
- CAIRO_MUTEX_UNLOCK (connection->shm_mutex);
+static void
+_cairo_xcb_shm_process_pending (cairo_xcb_connection_t *connection, shm_wait_type_t wait)
+{
+ cairo_xcb_shm_info_t *info, *next;
+ xcb_get_input_focus_reply_t *reply;
+
+ assert (CAIRO_MUTEX_IS_LOCKED (connection->shm_mutex));
+ cairo_list_foreach_entry_safe (info, next, cairo_xcb_shm_info_t,
+ &connection->shm_pending, pending)
+ {
+ switch (wait) {
+ case PENDING_WAIT:
+ reply = xcb_wait_for_reply (connection->xcb_connection,
+ info->sync.sequence, NULL);
+ break;
+ case PENDING_POLL:
+ if (! xcb_poll_for_reply (connection->xcb_connection,
+ info->sync.sequence,
+ (void **) &reply, NULL))
+ /* We cannot be sure the server finished with this image yet, so
+ * try again later. All other shm info are guranteed to have a
+ * larger sequence number and thus don't have to be checked. */
+ return;
+ break;
+ }
+
+ free (reply);
+ cairo_list_del (&info->pending);
+ _cairo_xcb_shm_info_finalize (info);
+ }
}
cairo_int_status_t
@@ -460,6 +495,8 @@ _cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,
assert (connection->flags & CAIRO_XCB_HAS_SHM);
CAIRO_MUTEX_LOCK (connection->shm_mutex);
+ _cairo_xcb_shm_process_pending (connection, PENDING_POLL);
+
cairo_list_foreach_entry_safe (pool, next, cairo_xcb_shm_mem_pool_t,
&connection->shm_pools, link)
{
@@ -547,6 +584,7 @@ _cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,
shm_info->shm = pool->shmseg;
shm_info->offset = (char *) mem - (char *) pool->base;
shm_info->mem = mem;
+ shm_info->sync.sequence = XCB_NONE;
/* scan for old, unused pools */
cairo_list_foreach_entry_safe (pool, next, cairo_xcb_shm_mem_pool_t,
@@ -565,8 +603,37 @@ _cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,
}
void
+_cairo_xcb_shm_info_destroy (cairo_xcb_shm_info_t *shm_info)
+{
+ cairo_xcb_connection_t *connection = shm_info->connection;
+
+ /* We can only return shm_info->mem to the allocator when we can be sure
+ * that the X server no longer reads from it. Since the X server processes
+ * requests in order, we send a GetInputFocus here.
+ * _cairo_xcb_shm_process_pending () will later check if the reply for that
+ * request was received and then actually mark this memory area as free. */
+
+ CAIRO_MUTEX_LOCK (connection->shm_mutex);
+ assert (shm_info->sync.sequence == XCB_NONE);
+ shm_info->sync = xcb_get_input_focus (connection->xcb_connection);
+
+ cairo_list_init (&shm_info->pending);
+ cairo_list_add_tail (&shm_info->pending, &connection->shm_pending);
+ CAIRO_MUTEX_UNLOCK (connection->shm_mutex);
+}
+
+void
+_cairo_xcb_connection_shm_mem_pools_flush (cairo_xcb_connection_t *connection)
+{
+ CAIRO_MUTEX_LOCK (connection->shm_mutex);
+ _cairo_xcb_shm_process_pending (connection, PENDING_WAIT);
+ CAIRO_MUTEX_UNLOCK (connection->shm_mutex);
+}
+
+void
_cairo_xcb_connection_shm_mem_pools_fini (cairo_xcb_connection_t *connection)
{
+ assert (cairo_list_is_empty (&connection->shm_pending));
while (! cairo_list_is_empty (&connection->shm_pools)) {
_cairo_xcb_shm_mem_pool_destroy (cairo_list_first_entry (&connection->shm_pools,
cairo_xcb_shm_mem_pool_t,
commit 6b4e07d1430c704fc976edf63c27c46f16a8751f
Author: Uli Schlachter <psychon at znc.in>
Date: Sat Dec 25 14:47:29 2010 +0100
Switch the order of two functions in the C file
The following commit adds a call to _cairo_xcb_shm_info_destroy to some function
in-between, but it also renames it and does some other changes to this. Thus,
move this function first to make the diff easier to read. :)
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-xcb-shm.c b/src/cairo-xcb-shm.c
index cfcf6d1..fae020a 100644
--- a/src/cairo-xcb-shm.c
+++ b/src/cairo-xcb-shm.c
@@ -412,6 +412,40 @@ _cairo_xcb_shm_mem_pool_destroy (cairo_xcb_shm_mem_pool_t *pool)
free (pool);
}
+void
+_cairo_xcb_shm_info_destroy (cairo_xcb_shm_info_t *shm_info)
+{
+ cairo_xcb_connection_t *connection = shm_info->connection;
+
+ CAIRO_MUTEX_LOCK (connection->shm_mutex);
+
+ _cairo_xcb_shm_mem_pool_free (shm_info->pool, shm_info->mem);
+ _cairo_freepool_free (&connection->shm_info_freelist, shm_info);
+
+ /* scan for old, unused pools - hold at least one in reserve */
+ if (! cairo_list_is_singular (&connection->shm_pools))
+ {
+ cairo_xcb_shm_mem_pool_t *pool, *next;
+ cairo_list_t head;
+
+ cairo_list_init (&head);
+ cairo_list_move (connection->shm_pools.next, &head);
+
+ cairo_list_foreach_entry_safe (pool, next, cairo_xcb_shm_mem_pool_t,
+ &connection->shm_pools, link)
+ {
+ if (pool->free_bytes == pool->max_bytes) {
+ _cairo_xcb_connection_shm_detach (connection, pool->shmseg);
+ _cairo_xcb_shm_mem_pool_destroy (pool);
+ }
+ }
+
+ cairo_list_move (head.next, &connection->shm_pools);
+ }
+
+ CAIRO_MUTEX_UNLOCK (connection->shm_mutex);
+}
+
cairo_int_status_t
_cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,
size_t size,
@@ -531,40 +565,6 @@ _cairo_xcb_connection_allocate_shm_info (cairo_xcb_connection_t *connection,
}
void
-_cairo_xcb_shm_info_destroy (cairo_xcb_shm_info_t *shm_info)
-{
- cairo_xcb_connection_t *connection = shm_info->connection;
-
- CAIRO_MUTEX_LOCK (connection->shm_mutex);
-
- _cairo_xcb_shm_mem_pool_free (shm_info->pool, shm_info->mem);
- _cairo_freepool_free (&connection->shm_info_freelist, shm_info);
-
- /* scan for old, unused pools - hold at least one in reserve */
- if (! cairo_list_is_singular (&connection->shm_pools))
- {
- cairo_xcb_shm_mem_pool_t *pool, *next;
- cairo_list_t head;
-
- cairo_list_init (&head);
- cairo_list_move (connection->shm_pools.next, &head);
-
- cairo_list_foreach_entry_safe (pool, next, cairo_xcb_shm_mem_pool_t,
- &connection->shm_pools, link)
- {
- if (pool->free_bytes == pool->max_bytes) {
- _cairo_xcb_connection_shm_detach (connection, pool->shmseg);
- _cairo_xcb_shm_mem_pool_destroy (pool);
- }
- }
-
- cairo_list_move (head.next, &connection->shm_pools);
- }
-
- CAIRO_MUTEX_UNLOCK (connection->shm_mutex);
-}
-
-void
_cairo_xcb_connection_shm_mem_pools_fini (cairo_xcb_connection_t *connection)
{
while (! cairo_list_is_empty (&connection->shm_pools)) {
commit 30b961f895f924ceb65574f15ecbe0ff1948c8aa
Author: Uli Schlachter <psychon at znc.in>
Date: Sat Dec 25 14:17:33 2010 +0100
Remove an unused field from cairo_xcb_shm_info_t
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-xcb-connection-shm.c b/src/cairo-xcb-connection-shm.c
index 3e76301..ccdcb27 100644
--- a/src/cairo-xcb-connection-shm.c
+++ b/src/cairo-xcb-connection-shm.c
@@ -46,7 +46,7 @@ _cairo_xcb_connection_shm_attach (cairo_xcb_connection_t *connection,
return segment;
}
-unsigned int
+void
_cairo_xcb_connection_shm_put_image (cairo_xcb_connection_t *connection,
xcb_drawable_t dst,
xcb_gcontext_t gc,
@@ -62,11 +62,9 @@ _cairo_xcb_connection_shm_put_image (cairo_xcb_connection_t *connection,
uint32_t shm,
uint32_t offset)
{
- xcb_void_cookie_t cookie;
- cookie = xcb_shm_put_image (connection->xcb_connection, dst, gc, total_width, total_height,
- src_x, src_y, width, height, dst_x, dst_y, depth,
- XCB_IMAGE_FORMAT_Z_PIXMAP, 0, shm, offset);
- return cookie.sequence;
+ xcb_shm_put_image (connection->xcb_connection, dst, gc, total_width, total_height,
+ src_x, src_y, width, height, dst_x, dst_y, depth,
+ XCB_IMAGE_FORMAT_Z_PIXMAP, 0, shm, offset);
}
cairo_status_t
diff --git a/src/cairo-xcb-private.h b/src/cairo-xcb-private.h
index c7f42f6..7da888c 100644
--- a/src/cairo-xcb-private.h
+++ b/src/cairo-xcb-private.h
@@ -66,7 +66,6 @@ struct _cairo_xcb_shm_info {
cairo_xcb_connection_t *connection;
uint32_t shm;
uint32_t offset;
- unsigned int seqno;
void *mem;
cairo_xcb_shm_mem_pool_t *pool;
};
@@ -534,7 +533,7 @@ _cairo_xcb_connection_shm_attach (cairo_xcb_connection_t *connection,
uint32_t id,
cairo_bool_t readonly);
-cairo_private unsigned int
+cairo_private void
_cairo_xcb_connection_shm_put_image (cairo_xcb_connection_t *connection,
xcb_drawable_t dst,
xcb_gcontext_t gc,
diff --git a/src/cairo-xcb-surface-core.c b/src/cairo-xcb-surface-core.c
index 4bcbd8f..b752a33 100644
--- a/src/cairo-xcb-surface-core.c
+++ b/src/cairo-xcb-surface-core.c
@@ -220,16 +220,15 @@ _pixmap_from_image (cairo_xcb_surface_t *target,
gc = _cairo_xcb_screen_get_gc (target->screen, pixmap->pixmap, image->depth);
if (shm_info != NULL) {
- shm_info->seqno =
- _cairo_xcb_connection_shm_put_image (target->connection,
- pixmap->pixmap, gc,
- image->width, image->height,
- 0, 0,
- image->width, image->height,
- 0, 0,
- image->depth,
- shm_info->shm,
- shm_info->offset);
+ _cairo_xcb_connection_shm_put_image (target->connection,
+ pixmap->pixmap, gc,
+ image->width, image->height,
+ 0, 0,
+ image->width, image->height,
+ 0, 0,
+ image->depth,
+ shm_info->shm,
+ shm_info->offset);
} else {
int len;
diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
index fb23fbe..760c90d 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -328,16 +328,15 @@ _picture_from_image (cairo_xcb_surface_t *target,
gc = _cairo_xcb_screen_get_gc (target->screen, pixmap, image->depth);
if (shm_info != NULL) {
- shm_info->seqno =
- _cairo_xcb_connection_shm_put_image (target->connection,
- pixmap, gc,
- image->width, image->height,
- 0, 0,
- image->width, image->height,
- 0, 0,
- image->depth,
- shm_info->shm,
- shm_info->offset);
+ _cairo_xcb_connection_shm_put_image (target->connection,
+ pixmap, gc,
+ image->width, image->height,
+ 0, 0,
+ image->width, image->height,
+ 0, 0,
+ image->depth,
+ shm_info->shm,
+ shm_info->offset);
} else {
int len;
diff --git a/src/cairo-xcb-surface.c b/src/cairo-xcb-surface.c
index 5ee79e0..6b45e8b 100644
--- a/src/cairo-xcb-surface.c
+++ b/src/cairo-xcb-surface.c
@@ -608,17 +608,16 @@ _put_shm_image (cairo_xcb_surface_t *surface,
if (shm_info == NULL)
return CAIRO_INT_STATUS_UNSUPPORTED;
- shm_info->seqno =
- _cairo_xcb_connection_shm_put_image (surface->connection,
- surface->drawable,
- gc,
- surface->width, surface->height,
- 0, 0,
- image->width, image->height,
- 0, 0,
- image->depth,
- shm_info->shm,
- shm_info->offset);
+ _cairo_xcb_connection_shm_put_image (surface->connection,
+ surface->drawable,
+ gc,
+ surface->width, surface->height,
+ 0, 0,
+ image->width, image->height,
+ 0, 0,
+ image->depth,
+ shm_info->shm,
+ shm_info->offset);
return CAIRO_STATUS_SUCCESS;
#else
More information about the cairo-commit
mailing list