Bug 50705 - cairo_glyph_path is not multithread safe
Summary: cairo_glyph_path is not multithread safe
Status: RESOLVED MOVED
Alias: None
Product: cairo
Classification: Unclassified
Component: general (show other bugs)
Version: 1.12.2
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-04 20:25 UTC by zadecn
Modified: 2018-08-25 13:30 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

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.