Commit 7eb33099 authored by Chris Wilson's avatar Chris Wilson 🤔
Browse files

snapshot: Perform the cow under a mutex



In order to prevent a race between concurrent destroy and use in another
thread, we need to acquire a reference to the snapshot->target under a
mutex. Whilst we hold that reference, it prevents the internal destroy
mechanism from freeing the memory we are using (if we have a pointer to
the original surface) and the client drops their final reference.

Oh boy, talk about opening a can of worms...
Signed-off-by: Chris Wilson's avatarChris Wilson <chris@chris-wilson.co.uk>
parent 455b4de1
......@@ -171,11 +171,7 @@ _analyze_recording_surface_pattern (cairo_analysis_surface_t *surface,
cairo_matrix_multiply (&tmp->ctm, &p2d, &surface->ctm);
tmp->has_ctm = ! _cairo_matrix_is_identity (&tmp->ctm);
if (_cairo_surface_is_snapshot (source))
source = _cairo_surface_snapshot_get_target (source);
if (_cairo_surface_is_subsurface (source))
source = _cairo_surface_subsurface_get_target (source);
source = _cairo_surface_get_source (source, NULL);
status = _cairo_recording_surface_replay_and_create_regions (source,
&tmp->base);
analysis_status = tmp->has_unsupported ? CAIRO_INT_STATUS_IMAGE_FALLBACK : CAIRO_INT_STATUS_SUCCESS;
......@@ -412,8 +408,7 @@ _cairo_analysis_surface_mask (void *abstract_surface,
if (source->type == CAIRO_PATTERN_TYPE_SURFACE) {
cairo_surface_t *src_surface = ((cairo_surface_pattern_t *)source)->surface;
if (_cairo_surface_is_snapshot (src_surface))
src_surface = _cairo_surface_snapshot_get_target (src_surface);
src_surface = _cairo_surface_get_source (src_surface, NULL);
if (_cairo_surface_is_recording (src_surface)) {
backend_source_status =
_analyze_recording_surface_pattern (surface, source);
......@@ -424,8 +419,7 @@ _cairo_analysis_surface_mask (void *abstract_surface,
if (mask->type == CAIRO_PATTERN_TYPE_SURFACE) {
cairo_surface_t *mask_surface = ((cairo_surface_pattern_t *)mask)->surface;
if (_cairo_surface_is_snapshot (mask_surface))
mask_surface = _cairo_surface_snapshot_get_target (mask_surface);
mask_surface = _cairo_surface_get_source (mask_surface, NULL);
if (_cairo_surface_is_recording (mask_surface)) {
backend_mask_status =
_analyze_recording_surface_pattern (surface, mask);
......
......@@ -432,6 +432,13 @@ _acquire_source_cleanup (pixman_image_t *pixman_image,
free (data);
}
static void
_defer_free_cleanup (pixman_image_t *pixman_image,
void *closure)
{
cairo_surface_destroy (closure);
}
static uint16_t
expand_channel (uint16_t v, uint32_t bits)
{
......@@ -816,11 +823,14 @@ _pixman_image_for_surface (cairo_image_surface_t *dst,
(! is_mask || ! pattern->base.has_component_alpha ||
(pattern->surface->content & CAIRO_CONTENT_COLOR) == 0))
{
cairo_surface_t *defer_free = NULL;
cairo_image_surface_t *source = (cairo_image_surface_t *) pattern->surface;
cairo_surface_type_t type;
if (_cairo_surface_is_snapshot (&source->base))
source = (cairo_image_surface_t *) _cairo_surface_snapshot_get_target (&source->base);
if (_cairo_surface_is_snapshot (&source->base)) {
defer_free = _cairo_surface_snapshot_get_target (&source->base);
source = (cairo_image_surface_t *) defer_free;
}
type = source->base.backend->type;
if (type == CAIRO_SURFACE_TYPE_IMAGE) {
......@@ -839,15 +849,19 @@ _pixman_image_for_surface (cairo_image_surface_t *dst,
sample->x >= source->width ||
sample->y >= source->height)
{
if (extend == CAIRO_EXTEND_NONE)
if (extend == CAIRO_EXTEND_NONE) {
cairo_surface_destroy (defer_free);
return _pixman_transparent_image ();
}
}
else
{
pixman_image = _pixel_to_solid (source,
sample->x, sample->y);
if (pixman_image)
if (pixman_image) {
cairo_surface_destroy (defer_free);
return pixman_image;
}
}
}
......@@ -858,6 +872,7 @@ _pixman_image_for_surface (cairo_image_surface_t *dst,
pattern->base.filter,
ix, iy))
{
cairo_surface_destroy (defer_free);
return pixman_image_ref (source->pixman_image);
}
#endif
......@@ -867,8 +882,16 @@ _pixman_image_for_surface (cairo_image_surface_t *dst,
source->height,
(uint32_t *) source->data,
source->stride);
if (unlikely (pixman_image == NULL))
if (unlikely (pixman_image == NULL)) {
cairo_surface_destroy (defer_free);
return NULL;
}
if (defer_free) {
pixman_image_set_destroy_function (pixman_image,
_defer_free_cleanup,
defer_free);
}
} else if (type == CAIRO_SURFACE_TYPE_SUBSURFACE) {
cairo_surface_subsurface_t *sub;
cairo_bool_t is_contained = FALSE;
......
......@@ -3668,9 +3668,7 @@ _cairo_pattern_get_ink_extents (const cairo_pattern_t *pattern,
(const cairo_surface_pattern_t *) pattern;
cairo_surface_t *surface = surface_pattern->surface;
if (_cairo_surface_is_snapshot (surface))
surface = _cairo_surface_snapshot_get_target (surface);
surface = _cairo_surface_get_source (surface, NULL);
if (_cairo_surface_is_recording (surface)) {
cairo_matrix_t imatrix;
cairo_box_t box;
......
......@@ -1221,23 +1221,28 @@ _get_source_surface_size (cairo_surface_t *source,
*width = extents->width;
*height = extents->height;
} else {
cairo_surface_t *free_me = NULL;
cairo_rectangle_int_t surf_extents;
cairo_box_t box;
cairo_bool_t bounded;
if (_cairo_surface_is_snapshot (source))
source = _cairo_surface_snapshot_get_target (source);
free_me = source = _cairo_surface_snapshot_get_target (source);
status = _cairo_recording_surface_get_ink_bbox ((cairo_recording_surface_t *)source,
&box, NULL);
if (unlikely (status))
if (unlikely (status)) {
cairo_surface_destroy (free_me);
return status;
_cairo_box_round_to_rectangle (&box, extents);
}
bounded = _cairo_surface_get_extents (source, &surf_extents);
cairo_surface_destroy (free_me);
*width = surf_extents.width;
*height = surf_extents.height;
_cairo_box_round_to_rectangle (&box, extents);
}
return CAIRO_STATUS_SUCCESS;
......@@ -2674,12 +2679,13 @@ _cairo_pdf_surface_emit_recording_surface (cairo_pdf_surface_t *surface,
cairo_box_double_t bbox;
cairo_int_status_t status;
int alpha = 0;
cairo_surface_t *free_me = NULL;
cairo_surface_t *source;
assert (pdf_source->type == CAIRO_PATTERN_TYPE_SURFACE);
source = pdf_source->surface;
if (_cairo_surface_is_snapshot (source))
source = _cairo_surface_snapshot_get_target (source);
free_me = source = _cairo_surface_snapshot_get_target (source);
old_width = surface->width;
old_height = surface->height;
......@@ -2700,12 +2706,12 @@ _cairo_pdf_surface_emit_recording_surface (cairo_pdf_surface_t *surface,
_get_bbox_from_extents (pdf_source->hash_entry->height, &pdf_source->hash_entry->extents, &bbox);
status = _cairo_pdf_surface_open_content_stream (surface, &bbox, &pdf_source->hash_entry->surface_res, TRUE);
if (unlikely (status))
return status;
goto err;
if (cairo_surface_get_content (source) == CAIRO_CONTENT_COLOR) {
status = _cairo_pdf_surface_add_alpha (surface, 1.0, &alpha);
if (unlikely (status))
return status;
goto err;
_cairo_output_stream_printf (surface->output,
"q /a%d gs 0 0 0 rg 0 0 %f %f re f Q\n",
......@@ -2720,7 +2726,7 @@ _cairo_pdf_surface_emit_recording_surface (cairo_pdf_surface_t *surface,
CAIRO_RECORDING_REGION_NATIVE);
assert (status != CAIRO_INT_STATUS_UNSUPPORTED);
if (unlikely (status))
return status;
goto err;
status = _cairo_pdf_surface_close_content_stream (surface);
......@@ -2731,6 +2737,8 @@ _cairo_pdf_surface_emit_recording_surface (cairo_pdf_surface_t *surface,
old_height);
surface->paginated_mode = old_paginated_mode;
err:
cairo_surface_destroy (free_me);
return status;
}
......
......@@ -1696,16 +1696,19 @@ _cairo_ps_surface_acquire_source_surface_from_pattern (cairo_ps_surface_t
*width = sub->extents.width;
*height = sub->extents.height;
} else {
cairo_surface_t *free_me = NULL;
cairo_recording_surface_t *recording_surface;
cairo_box_t bbox;
cairo_rectangle_int_t extents;
recording_surface = (cairo_recording_surface_t *) surf;
if (_cairo_surface_is_snapshot (&recording_surface->base))
recording_surface = (cairo_recording_surface_t *)
_cairo_surface_snapshot_get_target (&recording_surface->base);
if (_cairo_surface_is_snapshot (&recording_surface->base)) {
free_me = _cairo_surface_snapshot_get_target (&recording_surface->base);
recording_surface = (cairo_recording_surface_t *) free_me;
}
status = _cairo_recording_surface_get_bbox (recording_surface, &bbox, NULL);
cairo_surface_destroy (free_me);
if (unlikely (status))
return status;
......@@ -2848,6 +2851,7 @@ _cairo_ps_surface_emit_recording_surface (cairo_ps_surface_t *surface,
cairo_matrix_t old_cairo_to_ps;
cairo_content_t old_content;
cairo_rectangle_int_t old_page_bbox;
cairo_surface_t *free_me = NULL;
cairo_surface_clipper_t old_clipper;
cairo_box_t bbox;
cairo_int_status_t status;
......@@ -2862,14 +2866,14 @@ _cairo_ps_surface_emit_recording_surface (cairo_ps_surface_t *surface,
_cairo_ps_surface_clipper_intersect_clip_path);
if (_cairo_surface_is_snapshot (recording_surface))
recording_surface = _cairo_surface_snapshot_get_target (recording_surface);
free_me = recording_surface = _cairo_surface_snapshot_get_target (recording_surface);
status =
_cairo_recording_surface_get_bbox ((cairo_recording_surface_t *) recording_surface,
&bbox,
NULL);
if (unlikely (status))
return status;
goto err;
#if DEBUG_PS
_cairo_output_stream_printf (surface->stream,
......@@ -2907,11 +2911,11 @@ _cairo_ps_surface_emit_recording_surface (cairo_ps_surface_t *surface,
CAIRO_RECORDING_REGION_NATIVE);
assert (status != CAIRO_INT_STATUS_UNSUPPORTED);
if (unlikely (status))
return status;
goto err;
status = _cairo_pdf_operators_flush (&surface->pdf_operators);
if (unlikely (status))
return status;
goto err;
_cairo_output_stream_printf (surface->stream, " Q\n");
......@@ -2928,7 +2932,9 @@ _cairo_ps_surface_emit_recording_surface (cairo_ps_surface_t *surface,
_cairo_pdf_operators_set_cairo_to_pdf_matrix (&surface->pdf_operators,
&surface->cairo_to_ps);
return CAIRO_STATUS_SUCCESS;
err:
cairo_surface_destroy (free_me);
return status;
}
static cairo_int_status_t
......@@ -2941,6 +2947,7 @@ _cairo_ps_surface_emit_recording_subsurface (cairo_ps_surface_t *surface,
cairo_content_t old_content;
cairo_rectangle_int_t old_page_bbox;
cairo_surface_clipper_t old_clipper;
cairo_surface_t *free_me = NULL;
cairo_int_status_t status;
old_content = surface->content;
......@@ -2971,7 +2978,7 @@ _cairo_ps_surface_emit_recording_subsurface (cairo_ps_surface_t *surface,
_cairo_output_stream_printf (surface->stream, " q\n");
if (_cairo_surface_is_snapshot (recording_surface))
recording_surface = _cairo_surface_snapshot_get_target (recording_surface);
free_me = recording_surface = _cairo_surface_snapshot_get_target (recording_surface);
if (recording_surface->content == CAIRO_CONTENT_COLOR) {
surface->content = CAIRO_CONTENT_COLOR;
......@@ -2989,11 +2996,11 @@ _cairo_ps_surface_emit_recording_subsurface (cairo_ps_surface_t *surface,
CAIRO_RECORDING_REGION_NATIVE);
assert (status != CAIRO_INT_STATUS_UNSUPPORTED);
if (unlikely (status))
return status;
goto err;
status = _cairo_pdf_operators_flush (&surface->pdf_operators);
if (unlikely (status))
return status;
goto err;
_cairo_output_stream_printf (surface->stream, " Q\n");
......@@ -3010,7 +3017,9 @@ _cairo_ps_surface_emit_recording_subsurface (cairo_ps_surface_t *surface,
_cairo_pdf_operators_set_cairo_to_pdf_matrix (&surface->pdf_operators,
&surface->cairo_to_ps);
return CAIRO_STATUS_SUCCESS;
err:
cairo_surface_destroy (free_me);
return status;
}
static void
......
......@@ -1549,7 +1549,7 @@ _emit_surface_pattern (cairo_script_surface_t *surface,
{
cairo_script_context_t *ctx = to_context (surface);
cairo_surface_pattern_t *surface_pattern;
cairo_surface_t *source, *snapshot;
cairo_surface_t *source, *snapshot, *free_me = NULL;
cairo_surface_t *take_snapshot = NULL;
cairo_int_status_t status;
......@@ -1568,7 +1568,7 @@ _emit_surface_pattern (cairo_script_surface_t *surface,
if (_cairo_surface_snapshot_is_reused (source))
take_snapshot = source;
source = _cairo_surface_snapshot_get_target (source);
free_me = source = _cairo_surface_snapshot_get_target (source);
}
switch ((int) source->backend->type) {
......@@ -1585,6 +1585,7 @@ _emit_surface_pattern (cairo_script_surface_t *surface,
status = _emit_image_surface_pattern (surface, source);
break;
}
cairo_surface_destroy (free_me);
if (unlikely (status))
return status;
......
......@@ -47,7 +47,14 @@ _cairo_surface_snapshot_is_reused (cairo_surface_t *surface)
static inline cairo_surface_t *
_cairo_surface_snapshot_get_target (cairo_surface_t *surface)
{
return ((cairo_surface_snapshot_t *) surface)->target;
cairo_surface_snapshot_t *snapshot = (cairo_surface_snapshot_t *) surface;
cairo_surface_t *target;
CAIRO_MUTEX_LOCK (snapshot->mutex);
target = cairo_surface_reference (snapshot->target);
CAIRO_MUTEX_UNLOCK (snapshot->mutex);
return target;
}
static inline cairo_bool_t
......
......@@ -36,12 +36,14 @@
#ifndef CAIRO_SURFACE_SNAPSHOT_PRIVATE_H
#define CAIRO_SURFACE_SNAPSHOT_PRIVATE_H
#include "cairo-mutex-private.h"
#include "cairo-surface-private.h"
#include "cairo-surface-backend-private.h"
struct _cairo_surface_snapshot {
cairo_surface_t base;
cairo_mutex_t mutex;
cairo_surface_t *target;
cairo_surface_t *clone;
};
......
......@@ -149,6 +149,8 @@ _cairo_surface_snapshot_copy_on_write (cairo_surface_t *surface)
* been lost.
*/
CAIRO_MUTEX_LOCK (snapshot->mutex);
if (snapshot->target->backend->snapshot != NULL) {
clone = snapshot->target->backend->snapshot (snapshot->target);
if (clone != NULL) {
......@@ -165,7 +167,7 @@ _cairo_surface_snapshot_copy_on_write (cairo_surface_t *surface)
if (unlikely (status)) {
snapshot->target = _cairo_surface_create_in_error (status);
status = _cairo_surface_set_error (surface, status);
return;
goto unlock;
}
clone = image->base.backend->snapshot (&image->base);
_cairo_surface_release_source_image (snapshot->target, image, extra);
......@@ -174,6 +176,8 @@ done:
status = _cairo_surface_set_error (surface, clone->status);
snapshot->target = snapshot->clone = clone;
snapshot->base.type = clone->type;
unlock:
CAIRO_MUTEX_UNLOCK (snapshot->mutex);
}
/**
......@@ -230,6 +234,7 @@ _cairo_surface_snapshot (cairo_surface_t *surface)
surface->content);
snapshot->base.type = surface->type;
CAIRO_MUTEX_INIT (snapshot->mutex);
snapshot->target = surface;
snapshot->clone = NULL;
......
......@@ -302,7 +302,8 @@ _cairo_surface_subsurface_source (void *abstract_surface,
cairo_surface_t *source;
source = _cairo_surface_get_source (surface->target, extents);
*extents = surface->extents;
if (extents)
*extents = surface->extents;
return source;
}
......
......@@ -961,6 +961,8 @@ cairo_surface_finish (cairo_surface_t *surface)
/* We have to becareful when decoupling potential reference cycles */
cairo_surface_reference (surface);
_cairo_surface_finish (surface);
/* XXX need to block and wait for snapshot references */
cairo_surface_destroy (surface);
}
slim_hidden_def (cairo_surface_finish);
......@@ -1799,7 +1801,8 @@ cairo_surface_t *
_cairo_surface_default_source (void *surface,
cairo_rectangle_int_t *extents)
{
_cairo_surface_get_extents(surface, extents);
if (extents)
_cairo_surface_get_extents(surface, extents);
return surface;
}
......
......@@ -1127,10 +1127,7 @@ is_recording_pattern (const cairo_pattern_t *pattern)
return FALSE;
surface = ((const cairo_surface_pattern_t *) pattern)->surface;
if (_cairo_surface_is_paginated (surface))
surface = _cairo_paginated_surface_get_recording (surface);
if (_cairo_surface_is_snapshot (surface))
surface = _cairo_surface_snapshot_get_target (surface);
surface = _cairo_surface_get_source (surface, NULL);
return _cairo_surface_is_recording (surface);
}
......@@ -1140,11 +1137,7 @@ recording_pattern_get_surface (const cairo_pattern_t *pattern)
cairo_surface_t *surface;
surface = ((const cairo_surface_pattern_t *) pattern)->surface;
if (_cairo_surface_is_paginated (surface))
surface = _cairo_paginated_surface_get_recording (surface);
if (_cairo_surface_is_snapshot (surface))
surface = _cairo_surface_snapshot_get_target (surface);
return surface;
return _cairo_surface_get_source (surface, NULL);
}
static cairo_bool_t
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment