[cairo-commit] 3 commits - src/cairo-surface.c src/cairo-surface-inline.h src/cairo-surface-snapshot.c src/cairo-surface-snapshot-inline.h
Chris Wilson
ickle at kemper.freedesktop.org
Tue May 1 09:09:30 PDT 2012
src/cairo-surface-inline.h | 8 +++++
src/cairo-surface-snapshot-inline.h | 3 +-
src/cairo-surface-snapshot.c | 51 ++++++++++++++++++++++++++++++------
src/cairo-surface.c | 32 ++++++++++++++++------
4 files changed, 76 insertions(+), 18 deletions(-)
New commits:
commit db4ee947c3fc2c057dd8e84cdfcb779e7c62e5d5
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date: Tue May 1 14:41:25 2012 +0100
Split finish into multiple stages
In order to handle the snapshot copy-on-write losing a race with another
thread using the snapshot as a source, we may find the target acquires a
fresh reference as we attempt to finalize it.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index 46ba256..e16a354 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -155,6 +155,7 @@ static DEFINE_NIL_SURFACE(CAIRO_STATUS_DEVICE_ERROR, _cairo_surface_nil_device_e
static DEFINE_NIL_SURFACE(CAIRO_INT_STATUS_UNSUPPORTED, _cairo_surface_nil_unsupported);
static DEFINE_NIL_SURFACE(CAIRO_INT_STATUS_NOTHING_TO_DO, _cairo_surface_nil_nothing_to_do);
+static void _cairo_surface_finish_snapshots (cairo_surface_t *surface);
static void _cairo_surface_finish (cairo_surface_t *surface);
/**
@@ -855,11 +856,18 @@ cairo_surface_destroy (cairo_surface_t *surface)
assert (surface->snapshot_of == NULL);
- if (! surface->finished)
- _cairo_surface_finish (surface);
+ if (! surface->finished) {
+ _cairo_surface_finish_snapshots (surface);
+ /* We may have been referenced by a snapshot prior to have
+ * detaching it with the copy-on-write.
+ */
+ if (CAIRO_REFERENCE_COUNT_GET_VALUE (&surface->ref_count))
+ return;
- /* paranoid check that nobody took a reference whilst finishing */
- assert (! CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&surface->ref_count));
+ _cairo_surface_finish (surface);
+ /* paranoid check that nobody took a reference whilst finishing */
+ assert (! CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&surface->ref_count));
+ }
if (surface->damage)
_cairo_damage_destroy (surface->damage);
@@ -899,10 +907,8 @@ cairo_surface_get_reference_count (cairo_surface_t *surface)
}
static void
-_cairo_surface_finish (cairo_surface_t *surface)
+_cairo_surface_finish_snapshots (cairo_surface_t *surface)
{
- cairo_status_t status;
-
cairo_surface_flush (surface);
/* update the snapshots *before* we declare the surface as finished */
@@ -911,6 +917,12 @@ _cairo_surface_finish (cairo_surface_t *surface)
_cairo_surface_detach_snapshots (surface);
if (surface->snapshot_of != NULL)
_cairo_surface_detach_snapshot (surface);
+}
+
+static void
+_cairo_surface_finish (cairo_surface_t *surface)
+{
+ cairo_status_t status;
surface->finished = TRUE;
@@ -958,11 +970,13 @@ cairo_surface_finish (cairo_surface_t *surface)
if (surface->finished)
return;
- /* We have to becareful when decoupling potential reference cycles */
+ /* We have to be careful when decoupling potential reference cycles */
cairo_surface_reference (surface);
- _cairo_surface_finish (surface);
+ _cairo_surface_finish_snapshots (surface);
/* XXX need to block and wait for snapshot references */
+ _cairo_surface_finish (surface);
+
cairo_surface_destroy (surface);
}
slim_hidden_def (cairo_surface_finish);
commit 52dfa038b9e0c106aa3f9f08abeb7f53e72a762a
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date: Tue May 1 15:06:46 2012 +0100
snapshot: Avoid triggering assertion for grabbing the target during destroy
If the source wins the race to acquire the original surface as it is
being destroyed, it triggers an assertion.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
diff --git a/src/cairo-surface-inline.h b/src/cairo-surface-inline.h
index 1b257d5..27ea8f0 100644
--- a/src/cairo-surface-inline.h
+++ b/src/cairo-surface-inline.h
@@ -49,4 +49,12 @@ _cairo_surface_flush (cairo_surface_t *surface)
return status;
}
+static inline cairo_surface_t *
+_cairo_surface_reference (cairo_surface_t *surface)
+{
+ if (!CAIRO_REFERENCE_COUNT_IS_INVALID (&surface->ref_count))
+ _cairo_reference_count_inc (&surface->ref_count);
+ return surface;
+}
+
#endif /* CAIRO_SURFACE_INLINE_H */
diff --git a/src/cairo-surface-snapshot-inline.h b/src/cairo-surface-snapshot-inline.h
index 7c15b81..bf89c77 100644
--- a/src/cairo-surface-snapshot-inline.h
+++ b/src/cairo-surface-snapshot-inline.h
@@ -37,6 +37,7 @@
#define CAIRO_SURFACE_SNAPSHOT_INLINE_H
#include "cairo-surface-snapshot-private.h"
+#include "cairo-surface-inline.h"
static inline cairo_bool_t
_cairo_surface_snapshot_is_reused (cairo_surface_t *surface)
@@ -51,7 +52,7 @@ _cairo_surface_snapshot_get_target (cairo_surface_t *surface)
cairo_surface_t *target;
CAIRO_MUTEX_LOCK (snapshot->mutex);
- target = cairo_surface_reference (snapshot->target);
+ target = _cairo_surface_reference (snapshot->target);
CAIRO_MUTEX_UNLOCK (snapshot->mutex);
return target;
commit f62f8f907d14a7267f136f299208029c7b904eaa
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date: Tue May 1 14:57:18 2012 +0100
snapshot: Hold a reference to target whilst querying
Due to race with cow and accessing target from multiple threads, we need
to be careful that we always acquire a reference for our access to
the snapshot target.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
diff --git a/src/cairo-surface-snapshot.c b/src/cairo-surface-snapshot.c
index 7b8a9a1..9471e47 100644
--- a/src/cairo-surface-snapshot.c
+++ b/src/cairo-surface-snapshot.c
@@ -58,6 +58,8 @@ _cairo_surface_snapshot_finish (void *abstract_surface)
cairo_surface_destroy (surface->clone);
}
+ CAIRO_MUTEX_FINI (surface->mutex);
+
return status;
}
@@ -65,9 +67,15 @@ static cairo_status_t
_cairo_surface_snapshot_flush (void *abstract_surface)
{
cairo_surface_snapshot_t *surface = abstract_surface;
+ cairo_surface_t *target;
+ cairo_status_t status;
- cairo_surface_flush (surface->target);
- return surface->target->status;
+ target = _cairo_surface_snapshot_get_target (&surface->base);
+ cairo_surface_flush (target);
+ status = target->status;
+ cairo_surface_destroy (target);
+
+ return status;
}
static cairo_surface_t *
@@ -75,27 +83,48 @@ _cairo_surface_snapshot_source (void *abstract_surface,
cairo_rectangle_int_t *extents)
{
cairo_surface_snapshot_t *surface = abstract_surface;
- return _cairo_surface_get_source (surface->target, extents);
+ return _cairo_surface_get_source (surface->target, extents); /* XXX racy */
}
+struct snapshot_extra {
+ cairo_surface_t *target;
+ void *extra;
+};
+
static cairo_status_t
_cairo_surface_snapshot_acquire_source_image (void *abstract_surface,
cairo_image_surface_t **image_out,
void **extra_out)
{
cairo_surface_snapshot_t *surface = abstract_surface;
+ struct snapshot_extra *extra;
+ cairo_status_t status;
- return _cairo_surface_acquire_source_image (surface->target, image_out, extra_out);
+ extra = malloc (sizeof (*extra));
+ if (unlikely (extra == NULL))
+ return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+ extra->target = _cairo_surface_snapshot_get_target (&surface->base);
+ status = _cairo_surface_acquire_source_image (extra->target, image_out, &extra->extra);
+ if (unlikely (status)) {
+ cairo_surface_destroy (extra->target);
+ free (extra);
+ }
+
+ *extra_out = extra;
+ return status;
}
static void
_cairo_surface_snapshot_release_source_image (void *abstract_surface,
cairo_image_surface_t *image,
- void *extra)
+ void *_extra)
{
- cairo_surface_snapshot_t *surface = abstract_surface;
+ struct snapshot_extra *extra = _extra;
- _cairo_surface_release_source_image (surface->target, image, extra);
+ _cairo_surface_release_source_image (extra->target, image, extra->extra);
+ cairo_surface_destroy (extra->target);
+ free (extra);
}
static cairo_bool_t
@@ -103,8 +132,14 @@ _cairo_surface_snapshot_get_extents (void *abstract_surface,
cairo_rectangle_int_t *extents)
{
cairo_surface_snapshot_t *surface = abstract_surface;
+ cairo_surface_t *target;
+ cairo_bool_t bounded;
+
+ target = _cairo_surface_snapshot_get_target (&surface->base);
+ bounded = _cairo_surface_get_extents (target, extents);
+ cairo_surface_destroy (target);
- return _cairo_surface_get_extents (surface->target, extents);
+ return bounded;
}
static const cairo_surface_backend_t _cairo_surface_snapshot_backend = {
More information about the cairo-commit
mailing list