Bug 50705

Summary:	cairo_glyph_path is not multithread safe
Product:	cairo	Reporter:	zadecn <zhaohongchao>
Component:	general	Assignee:	Carl Worth <cworth>
Status:	RESOLVED MOVED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	normal
Priority:	medium	CC:	freedesktop
Version:	1.12.2
Hardware:	x86-64 (AMD64)
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:

Description zadecn 2012-06-04 20:25:20 UTC

environment: cairo on red hat linux with GCC 3.4.5.
context: use cairo_glyph_path to get halo effect.

description:

When I call the function in single thread environment, it is no problem; but in multihread when run more than 2 hours, it crashes with the GDB info:


#0  0x0000000000b1f899 in kill ()
#1  0x00000000007f4d7b in __pthread_raise () at src/ul_file.cpp:28
#2  0x0000000000b1fc63 in abort ()
#3  0x0000000000b1a39c in __assert_fail ()
#4  0x00000000006f1589 in _cairo_hash_table_remove (hash_table=Variable "hash_table" is not available.
) at cairo-hash.c:506
#5  0x00000000006d4b82 in _cairo_scaled_glyph_page_destroy (closure=Variable "closure" is not available.
) at cairo-scaled-font.c:461
#6  0x00000000006ed1ac in _cairo_cache_shrink_to_accommodate (cache=0x1426480, additional=0) at cairo-cache.c:223
#7  0x00000000006d5049 in _cairo_scaled_font_thaw_cache (scaled_font=0x2acb989780) at cairo-scaled-font.c:793
#8  0x00000000006d6a66 in _cairo_scaled_font_glyph_path (scaled_font=0x2acb989780, glyphs=0x2aab8358b0, num_glyphs=1, path=0x2acc20e9f0) at cairo-scaled-font.c:2635
#9  0x00000000006bb26d in _cairo_gstate_glyph_path (gstate=0x2acc20e6b8, glyphs=0x2acc76e210, num_glyphs=1, path=0x2acc20e9f0) at cairo-gstate.c:2061
#10 0x00000000006b2ce6 in cairo_glyph_path (cr=0x2acc20e680, glyphs=Variable "glyphs" is not available.
) at cairo.c:3548
#11 0x000000000065366b in map::(anonymous namespace)::DrawTextImpl (cr=0x2acc20e680, scaled_font=0x2ac43638e0, text=@0x2acc76e250, at=@0x2acc21fb30, desc=@0x1585ad0, angle=0.17453292519943295) at render_cairo.cpp:282
#12 0x0000000000654ae4 in map::CairoRender::DrawText (this=0x2ab0023e30, pts=0x2acc21fb30, count=Variable "count" is not available.

On my more than 10 cores, it is on the same function: cairo_glyph_path. Additionally, when I see other threads info, at least one thread run the same   function.

so my question is: is cairo_glyph_path multithread safe ?

Comment 1 Chris Wilson 2012-06-05 00:05:32 UTC

Silly question, can you please test:

diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
index 59440b2..2f4dbfe 100644
--- a/src/cairo-scaled-font.c
+++ b/src/cairo-scaled-font.c
@@ -2861,8 +2861,10 @@ _cairo_scaled_font_free_last_glyph (cairo_scaled_font_t *scaled_font,
     _cairo_scaled_glyph_fini (scaled_font, scaled_glyph);
 
     if (--page->num_glyphs == 0) {
+	CAIRO_MUTEX_LOCK (_cairo_scaled_glyph_page_cache_mutex);
 	_cairo_cache_remove (&cairo_scaled_glyph_page_cache,
 		             &page->cache_entry);
+	CAIRO_MUTEX_UNLOCK (_cairo_scaled_glyph_page_cache_mutex);
     }
 }

Comment 2 zadecn 2012-06-05 23:17:11 UTC

(In reply to comment #1)
> Silly question, can you please test:
> 
> diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
> index 59440b2..2f4dbfe 100644
> --- a/src/cairo-scaled-font.c
> +++ b/src/cairo-scaled-font.c
> @@ -2861,8 +2861,10 @@ _cairo_scaled_font_free_last_glyph (cairo_scaled_font_t
> *scaled_font,
>      _cairo_scaled_glyph_fini (scaled_font, scaled_glyph);
> 
>      if (--page->num_glyphs == 0) {
> +    CAIRO_MUTEX_LOCK (_cairo_scaled_glyph_page_cache_mutex);
>      _cairo_cache_remove (&cairo_scaled_glyph_page_cache,
>                       &page->cache_entry);
> +    CAIRO_MUTEX_UNLOCK (_cairo_scaled_glyph_page_cache_mutex);
>      }
>  }

add lock will avoid core? why not add directly other than a patch?

Comment 3 zadecn 2012-06-07 01:29:30 UTC

(In reply to comment #1)
> Silly question, can you please test:
> 
> diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
> index 59440b2..2f4dbfe 100644
> --- a/src/cairo-scaled-font.c
> +++ b/src/cairo-scaled-font.c
> @@ -2861,8 +2861,10 @@ _cairo_scaled_font_free_last_glyph (cairo_scaled_font_t
> *scaled_font,
>      _cairo_scaled_glyph_fini (scaled_font, scaled_glyph);
> 
>      if (--page->num_glyphs == 0) {
> +    CAIRO_MUTEX_LOCK (_cairo_scaled_glyph_page_cache_mutex);
>      _cairo_cache_remove (&cairo_scaled_glyph_page_cache,
>                       &page->cache_entry);
> +    CAIRO_MUTEX_UNLOCK (_cairo_scaled_glyph_page_cache_mutex);
>      }
>  }
I test it with the lock, but it cores also

Comment 4 Uli Schlachter 2013-08-21 13:09:37 UTC

Hey Behdad, could you take a look at this? AFAIK you looked into thread-safety for text handling.

Comment 5 Behdad Esfahbod 2013-08-21 20:08:07 UTC

Cairo scaled-font cache locking is too complex for me to fully understand these days.  ickle is the only one who still understands that code.

Comment 6 GitLab Migration User 2018-08-25 13:30:29 UTC

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/39.

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.