Attachment 134722 Details for Bug 103037 – Surround initialisations with atomic critical section

[patch] Surround initialisations with atomic critical section

0001-Surround-initialisations-with-atomic-critical-sectio.patch (text/plain), 14.20 KB, created by Mikhail Fludkov on 2017-10-07 09:29:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mikhail Fludkov

Created: 2017-10-07 09:29:23 UTC

Size: 14.20 KB

patch

obsolete

>From 66633ff1854fe7aeb29b00ae4a742e391628f21c Mon Sep 17 00:00:00 2001
>From: Mikhail Fludkov <misha@pexip.com>
>Date: Fri, 6 Oct 2017 13:47:51 +0200
>Subject: [PATCH] Surround initialisations with atomic critical section
>
>Fixes the race condition when one thread uses cairo_mask_compositor_t
>pointer returned by _cairo_image_mask_compositor_get, while another one
>started but has not finished it's initialisation yet
>
>Usage:
>static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>if (_cairo_atomic_init_once_enter(&once)) {
>    /* Initialization code */
>    _cairo_atomic_init_once_leave(&once);
>}
>
>https://bugs.freedesktop.org/show_bug.cgi?id=103037
>---
> src/cairo-atomic-private.h             | 33 +++++++++++++++++++++++++++++++++
> src/cairo-gl-msaa-compositor.c         |  5 ++++-
> src/cairo-gl-spans-compositor.c        |  5 ++++-
> src/cairo-gl-traps-compositor.c        |  5 ++++-
> src/cairo-image-compositor.c           | 19 ++++++++++++++-----
> src/cairo-image-mask-compositor.c      |  5 ++++-
> src/cairo-xlib-core-compositor.c       |  5 ++++-
> src/cairo-xlib-render-compositor.c     | 10 ++++++++--
> src/test-null-compositor-surface.c     | 10 ++++++++--
> src/win32/cairo-win32-gdi-compositor.c |  5 ++++-
> 10 files changed, 87 insertions(+), 15 deletions(-)
>
>diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h
>index 500129039..723a326ce 100644
>--- a/src/cairo-atomic-private.h
>+++ b/src/cairo-atomic-private.h
>@@ -45,6 +45,8 @@
> #include "config.h"
> #endif
> 
>+#include <assert.h>
>+
> /* The autoconf on OpenBSD 4.5 produces the malformed constant name
>  * SIZEOF_VOID__ rather than SIZEOF_VOID_P.  Work around that here. */
> #if !defined(SIZEOF_VOID_P) && defined(SIZEOF_VOID__)
>@@ -393,6 +395,37 @@ _cairo_atomic_ptr_cmpxchg_return_old_fallback(void **x, void *oldv, void *newv)
>     (void) ret__; \
> } while (0)
> 
>+typedef cairo_atomic_int_t cairo_atomic_once_t;
>+
>+#define CAIRO_ATOMIC_ONCE_UNINITIALIZED (0)
>+#define CAIRO_ATOMIC_ONCE_INITIALIZING  (1)
>+#define CAIRO_ATOMIC_ONCE_INITIALIZED   (2)
>+#define CAIRO_ATOMIC_ONCE_INIT          CAIRO_ATOMIC_ONCE_UNINITIALIZED
>+
>+static cairo_always_inline cairo_bool_t
>+_cairo_atomic_init_once_enter(cairo_atomic_once_t *once)
>+{
>+    if (likely(_cairo_atomic_int_get(once) == CAIRO_ATOMIC_ONCE_INITIALIZED))
>+	return 0;
>+
>+    if (_cairo_atomic_int_cmpxchg(once,
>+				  CAIRO_ATOMIC_ONCE_UNINITIALIZED,
>+				  CAIRO_ATOMIC_ONCE_INITIALIZING))
>+	return 1;
>+
>+    while (_cairo_atomic_int_get(once) != CAIRO_ATOMIC_ONCE_INITIALIZED) {}
>+    return 0;
>+}
>+
>+static cairo_always_inline void
>+_cairo_atomic_init_once_leave(cairo_atomic_once_t *once)
>+{
>+    if (unlikely(!_cairo_atomic_int_cmpxchg(once,
>+					    CAIRO_ATOMIC_ONCE_INITIALIZING,
>+					    CAIRO_ATOMIC_ONCE_INITIALIZED)))
>+	assert (0 && "incorrect use of _cairo_atomic_init_once API (once != CAIRO_ATOMIC_ONCE_INITIALIZING)");
>+}
>+
> CAIRO_END_DECLS
> 
> #endif
>diff --git a/src/cairo-gl-msaa-compositor.c b/src/cairo-gl-msaa-compositor.c
>index f9cd7c296..00150df0c 100644
>--- a/src/cairo-gl-msaa-compositor.c
>+++ b/src/cairo-gl-msaa-compositor.c
>@@ -944,10 +944,13 @@ _cairo_gl_msaa_compositor_init (cairo_compositor_t	 *compositor,
> const cairo_compositor_t *
> _cairo_gl_msaa_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_compositor_t compositor;
>-    if (compositor.delegate == NULL)
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_gl_msaa_compositor_init (&compositor,
> 					_cairo_gl_span_compositor_get ());
>+	_cairo_atomic_init_once_leave(&once);
>+    }
> 
>     return &compositor;
> }
>diff --git a/src/cairo-gl-spans-compositor.c b/src/cairo-gl-spans-compositor.c
>index 4317ccd07..46ddc9826 100644
>--- a/src/cairo-gl-spans-compositor.c
>+++ b/src/cairo-gl-spans-compositor.c
>@@ -528,10 +528,11 @@ _cairo_gl_span_renderer_fini (cairo_abstract_span_renderer_t *_r,
> const cairo_compositor_t *
> _cairo_gl_span_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_spans_compositor_t spans;
>     static cairo_compositor_t shape;
> 
>-    if (spans.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	/* The fallback to traps here is essentially just for glyphs... */
> 	_cairo_shape_mask_compositor_init (&shape,
> 					   _cairo_gl_traps_compositor_get());
>@@ -547,6 +548,8 @@ _cairo_gl_span_compositor_get (void)
> 	//spans.check_span_renderer = check_span_renderer;
> 	spans.renderer_init = _cairo_gl_span_renderer_init;
> 	spans.renderer_fini = _cairo_gl_span_renderer_fini;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &spans.base;
>diff --git a/src/cairo-gl-traps-compositor.c b/src/cairo-gl-traps-compositor.c
>index 1e11006ed..928f0189f 100644
>--- a/src/cairo-gl-traps-compositor.c
>+++ b/src/cairo-gl-traps-compositor.c
>@@ -500,9 +500,10 @@ check_composite (const cairo_composite_rectangles_t *extents)
> const cairo_compositor_t *
> _cairo_gl_traps_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_traps_compositor_t compositor;
> 
>-    if (compositor.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_traps_compositor_init (&compositor, &_cairo_fallback_compositor);
> 	compositor.acquire = acquire;
> 	compositor.release = release;
>@@ -522,6 +523,8 @@ _cairo_gl_traps_compositor_get (void)
> 	compositor.composite_tristrip = composite_tristrip;
> 	compositor.check_composite_glyphs = _cairo_gl_check_composite_glyphs;
> 	compositor.composite_glyphs = _cairo_gl_composite_glyphs;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor.base;
>diff --git a/src/cairo-image-compositor.c b/src/cairo-image-compositor.c
>index bfa1e92d4..2d25a31bb 100644
>--- a/src/cairo-image-compositor.c
>+++ b/src/cairo-image-compositor.c
>@@ -1244,11 +1244,12 @@ check_composite (const cairo_composite_rectangles_t *extents)
> const cairo_compositor_t *
> _cairo_image_traps_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_traps_compositor_t compositor;
> 
>-    if (compositor.base.delegate == NULL) {
>-	_cairo_traps_compositor_init (&compositor,
>-				      &__cairo_no_compositor);
>+    if (_cairo_atomic_init_once_enter(&once)) {
>+	_cairo_traps_compositor_init(&compositor,
>+				     &__cairo_no_compositor);
> 	compositor.acquire = acquire;
> 	compositor.release = release;
> 	compositor.set_clip_region = set_clip_region;
>@@ -1269,6 +1270,8 @@ _cairo_image_traps_compositor_get (void)
> #endif
> 	compositor.check_composite_glyphs = check_composite_glyphs;
> 	compositor.composite_glyphs = composite_glyphs;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor.base;
>@@ -1277,9 +1280,10 @@ _cairo_image_traps_compositor_get (void)
> const cairo_compositor_t *
> _cairo_image_mask_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_mask_compositor_t compositor;
> 
>-    if (compositor.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_mask_compositor_init (&compositor,
> 				     _cairo_image_traps_compositor_get ());
> 	compositor.acquire = acquire;
>@@ -1296,6 +1300,8 @@ _cairo_image_mask_compositor_get (void)
> 	compositor.composite_boxes = composite_boxes;
> 	compositor.check_composite_glyphs = check_composite_glyphs;
> 	compositor.composite_glyphs = composite_glyphs;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor.base;
>@@ -3105,10 +3111,11 @@ span_renderer_fini (cairo_abstract_span_renderer_t *_r,
> const cairo_compositor_t *
> _cairo_image_spans_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_spans_compositor_t spans;
>     static cairo_compositor_t shape;
> 
>-    if (spans.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_shape_mask_compositor_init (&shape,
> 					   _cairo_image_traps_compositor_get());
> 	shape.glyphs = NULL;
>@@ -3131,6 +3138,8 @@ _cairo_image_spans_compositor_get (void)
> 	//spans.check_span_renderer = check_span_renderer;
> 	spans.renderer_init = span_renderer_init;
> 	spans.renderer_fini = span_renderer_fini;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &spans.base;
>diff --git a/src/cairo-image-mask-compositor.c b/src/cairo-image-mask-compositor.c
>index bb990dd65..feb7c6222 100644
>--- a/src/cairo-image-mask-compositor.c
>+++ b/src/cairo-image-mask-compositor.c
>@@ -387,9 +387,10 @@ composite_boxes (void			*_dst,
> const cairo_compositor_t *
> _cairo_image_mask_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_mask_compositor_t compositor;
> 
>-    if (compositor.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_mask_compositor_init (&compositor,
> 				     _cairo_image_traps_compositor_get ());
> 	compositor.acquire = acquire;
>@@ -405,6 +406,8 @@ _cairo_image_mask_compositor_get (void)
> 	compositor.composite = composite;
> 	//compositor.check_composite_boxes = check_composite_boxes;
> 	compositor.composite_boxes = composite_boxes;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor.base;
>diff --git a/src/cairo-xlib-core-compositor.c b/src/cairo-xlib-core-compositor.c
>index 5babcc81b..139299961 100644
>--- a/src/cairo-xlib-core-compositor.c
>+++ b/src/cairo-xlib-core-compositor.c
>@@ -632,9 +632,10 @@ _cairo_xlib_core_compositor_fill (const cairo_compositor_t	*compositor,
> const cairo_compositor_t *
> _cairo_xlib_core_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_compositor_t compositor;
> 
>-    if (compositor.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	compositor.delegate = _cairo_xlib_fallback_compositor_get ();
> 
> 	compositor.paint = _cairo_xlib_core_compositor_paint;
>@@ -642,6 +643,8 @@ _cairo_xlib_core_compositor_get (void)
> 	compositor.fill  = _cairo_xlib_core_compositor_fill;
> 	compositor.stroke = _cairo_xlib_core_compositor_stroke;
> 	compositor.glyphs = NULL; /* XXX PolyGlyph? */
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor;
>diff --git a/src/cairo-xlib-render-compositor.c b/src/cairo-xlib-render-compositor.c
>index 861ebe638..4352b44c1 100644
>--- a/src/cairo-xlib-render-compositor.c
>+++ b/src/cairo-xlib-render-compositor.c
>@@ -1725,9 +1725,10 @@ composite_glyphs (void				*surface,
> const cairo_compositor_t *
> _cairo_xlib_mask_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_mask_compositor_t compositor;
> 
>-    if (compositor.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_mask_compositor_init (&compositor,
> 				     _cairo_xlib_fallback_compositor_get ());
> 
>@@ -1745,6 +1746,8 @@ _cairo_xlib_mask_compositor_get (void)
> 	compositor.composite_boxes = composite_boxes;
> 	compositor.check_composite_glyphs = check_composite_glyphs;
> 	compositor.composite_glyphs = composite_glyphs;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor.base;
>@@ -1973,9 +1976,10 @@ composite_tristrip (void		*abstract_dst,
> const cairo_compositor_t *
> _cairo_xlib_traps_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_traps_compositor_t compositor;
> 
>-    if (compositor.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_traps_compositor_init (&compositor,
> 				      _cairo_xlib_mask_compositor_get ());
> 
>@@ -1997,6 +2001,8 @@ _cairo_xlib_traps_compositor_get (void)
> 	compositor.composite_tristrip = composite_tristrip;
> 	compositor.check_composite_glyphs = check_composite_glyphs;
> 	compositor.composite_glyphs = composite_glyphs;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor.base;
>diff --git a/src/test-null-compositor-surface.c b/src/test-null-compositor-surface.c
>index d5d98c88f..6b8d4eeb6 100644
>--- a/src/test-null-compositor-surface.c
>+++ b/src/test-null-compositor-surface.c
>@@ -404,9 +404,10 @@ check_composite (const cairo_composite_rectangles_t *extents)
> static const cairo_compositor_t *
> no_traps_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_traps_compositor_t compositor;
> 
>-    if (compositor.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_traps_compositor_init (&compositor,
> 				      no_fallback_compositor_get ());
> 
>@@ -426,6 +427,8 @@ no_traps_compositor_get (void)
> 	compositor.composite_traps = composite_traps;
> 	compositor.check_composite_glyphs = check_composite_glyphs;
> 	compositor.composite_glyphs = composite_glyphs;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor.base;
>@@ -434,9 +437,10 @@ no_traps_compositor_get (void)
> static const cairo_compositor_t *
> no_spans_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_spans_compositor_t compositor;
> 
>-    if (compositor.base.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	_cairo_spans_compositor_init (&compositor,
> 				      no_traps_compositor_get());
> 
>@@ -448,6 +452,8 @@ no_spans_compositor_get (void)
> 	//compositor.check_span_renderer = check_span_renderer;
> 	compositor.renderer_init = span_renderer_init;
> 	compositor.renderer_fini = span_renderer_fini;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor.base;
>diff --git a/src/win32/cairo-win32-gdi-compositor.c b/src/win32/cairo-win32-gdi-compositor.c
>index 073e889ab..2858affcb 100644
>--- a/src/win32/cairo-win32-gdi-compositor.c
>+++ b/src/win32/cairo-win32-gdi-compositor.c
>@@ -634,9 +634,10 @@ _cairo_win32_gdi_compositor_glyphs (const cairo_compositor_t	*compositor,
> const cairo_compositor_t *
> _cairo_win32_gdi_compositor_get (void)
> {
>+    static cairo_atomic_once_t once = CAIRO_ATOMIC_ONCE_INIT;
>     static cairo_compositor_t compositor;
> 
>-    if (compositor.delegate == NULL) {
>+    if (_cairo_atomic_init_once_enter(&once)) {
> 	compositor.delegate = &_cairo_fallback_compositor;
> 
> 	compositor.paint  = _cairo_win32_gdi_compositor_paint;
>@@ -644,6 +645,8 @@ _cairo_win32_gdi_compositor_get (void)
> 	compositor.fill   = _cairo_win32_gdi_compositor_fill;
> 	compositor.stroke = _cairo_win32_gdi_compositor_stroke;
> 	compositor.glyphs = _cairo_win32_gdi_compositor_glyphs;
>+
>+	_cairo_atomic_init_once_leave(&once);
>     }
> 
>     return &compositor;
>-- 
>2.14.2
>

Actions: View | Splinter Review

Attachments on bug 103037: 134564 | 134702 | 134709 | 134722

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.