Bug 4299 - Assertion fails in "cairo-font.c" when using multithreads
Summary: Assertion fails in "cairo-font.c" when using multithreads
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: general (show other bugs)
Version: 1.0.0
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
: 4300 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-08-30 01:34 UTC by Alexey Shabalin
Modified: 2008-10-10 08:09 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Input SVG-file for this test application (132.77 KB, text/plain)
2005-09-01 03:04 UTC, Alexey Shabalin
Details
My C++ test application to demonstrate the bug (8.49 KB, text/plain)
2005-09-01 03:05 UTC, Alexey Shabalin
Details
Makefile for build this test (1.37 KB, text/plain)
2005-09-01 03:05 UTC, Alexey Shabalin
Details
Simpler test case for bug (2.46 KB, text/plain)
2005-09-08 12:29 UTC, Carl Worth
Details
Initial proposed patch (6.36 KB, patch)
2005-09-12 17:16 UTC, Carl Worth
Details | Splinter Review
HEAD patch 1/2: Fix locking in cairo_scaled_font_reference/destroy (5.13 KB, patch)
2005-09-13 10:22 UTC, Carl Worth
Details | Splinter Review
HEAD patch 2/2: Add missing locking to _cairo_scaled_glyph_lookup (2.66 KB, patch)
2005-09-13 10:23 UTC, Carl Worth
Details | Splinter Review
BRANCH_1_0 patch 1/2: Fix locking in cairo_scaled_font_reference/destroy (4.40 KB, patch)
2005-09-13 15:39 UTC, Carl Worth
Details | Splinter Review
BRANCH_1_0 patch 2/2: Fix locking in _cairo_ft_scaled_font_text_to_glyphs (2.52 KB, patch)
2005-09-13 15:42 UTC, Carl Worth
Details | Splinter Review
Corrected makefile (1.40 KB, text/plain)
2005-09-15 00:16 UTC, Alexey Shabalin
Details
Corrected cpp test file (8.48 KB, text/plain)
2005-09-15 00:17 UTC, Alexey Shabalin
Details

Description Alexey Shabalin 2005-08-30 01:34:18 UTC
I got an error message from my Java application that using Cairo:
"cairo-font.c:788: cairo_scaled_font_reference: Assertion `i < font_map-
>num_holdovers' failed."
This error is generated when I'm using libcairo release 1.0.0.

If using older libcairo build (0.4.0) I got another messages:
An unexpected exception has been detected in native code outside the VM.
Unexpected Signal : 11 occurred at PC=0xA9ACB346
Function=pixman_region_num_rects+0x12
Library=../../Bin/libpixman.so.1
-----------------------------------
java: cairo_ft_font.c:402: _ft_unscaled_font_unlock_face: Assertion `unscaled-
>lock > 0' failed.
-----------------
An unexpected exception has been detected in native code outside the VM.
Unexpected Signal : 11 occurred at PC=0xA9BBFA01
Function=FcFontSetMatch+0x241
Library=/usr/lib/libfontconfig.so.1
-------------------------------------
An unexpected exception has been detected in native code outside the VM.
Unexpected Signal : 11 occurred at PC=0x9393781
Function=[Unknown.]
Library=(N/A)

I switched my application to single-thread mode all is working fine.
If I put a lock on call of svg_renderer in my code the crash disappear too.

This is a part of my code:

"
void svg2image(NavMapImage *src, NavMapImage *target, cairo_format_t format, 
               void (*write_fn)(char * buffer, int width, int height, 
NavMapImage *target)
               ){
    unsigned int svg_width, svg_height;


    CAIRO_MUTEX_DECLARE(my_cairo_mutex);
    
    svg_cairo_status_t status;
    cairo_t *cr;
    svg_cairo_t *svgc;
    cairo_surface_t *surface;

    status = svg_cairo_create (&svgc);

    if (status) {
	    fprintf (stderr, "Failed to create svg_cairo_t. Exiting.\n");
	    return;
    }

        status = svg_cairo_parse_buffer(svgc, src->data, src->size);
        if (status)
	        return;

        svg_cairo_get_size (svgc, &svg_width, &svg_height);
        int stride;

        switch(format) {
            case CAIRO_FORMAT_A1:
                stride = svg_width / 8;
                break;
            case CAIRO_FORMAT_A8:
                stride = svg_width;
                break;
            case CAIRO_FORMAT_RGB24:
            case CAIRO_FORMAT_ARGB32:
            default:
                stride = CAIRO_BPP * svg_width;
        }   

        char * image_buf = (char *) malloc (stride * svg_height);

        surface = cairo_image_surface_create_for_data ((unsigned char *) 
image_buf, format, svg_width, svg_height, stride);
            cr = cairo_create (surface);
  
            /// XXX: This probably doesn't need to be here (eventually) 
                cairo_set_source_rgb (cr, 1, 1, 1);

                CAIRO_MUTEX_LOCK(my_cairo_mutex);
                    status = svg_cairo_render (svgc, cr);
                CAIRO_MUTEX_UNLOCK(my_cairo_mutex);

            cairo_destroy (cr);
        
        cairo_surface_destroy(surface);

    svg_cairo_destroy (svgc);
   
     // call function to write image to corresponding format
    (*write_fn) (image_buf, svg_width, svg_height, target);

    free (image_buf);
}
"
Comment 1 Owen Taylor 2005-08-30 05:24:41 UTC
*** Bug 4300 has been marked as a duplicate of this bug. ***
Comment 2 Alexey Shabalin 2005-09-01 03:04:23 UTC
Created attachment 3146 [details]
Input SVG-file for this test application
Comment 3 Alexey Shabalin 2005-09-01 03:05:20 UTC
Created attachment 3147 [details]
My C++ test application to demonstrate the bug
Comment 4 Alexey Shabalin 2005-09-01 03:05:58 UTC
Created attachment 3148 [details]
Makefile for build this test
Comment 5 Alexey Shabalin 2005-09-01 03:08:24 UTC
Comment on attachment 3148 [details]
Makefile for build this test

You will also need to create two directories "cairo" and "libs" and put there
necessary Cairo's headers and libraries accordingly for compiling and linking.
Comment 6 Carl Worth 2005-09-08 12:29:43 UTC
Created attachment 3202 [details]
Simpler test case for bug

Thanks for the bug report.

I've verified that this is indeed a bug. I've attached here a simpler test
case for the bug, (which depends on only cairo, needs no external SVG image,
etc.)
Comment 7 Carl Worth 2005-09-12 17:16:48 UTC
Created attachment 3243 [details] [review]
Initial proposed patch

I've found three locking bugs so far that are all triggered by this test case.

The attached patch fixes those three. I'll document it a bit better before I
commit it, but I thought you might want to do some testing with it.
Comment 8 Carl Worth 2005-09-12 17:29:10 UTC
It's possible that I posted the wrong version of the patch.

(I did have a version that was working, but that may have been a just-earlier
version of the patch, and I may be seeing deadlocks now with what I attached here.)

Just an FYI in case you do some testing. I'll sort things out and provide a
tested, documented patch soon, (hopefully tomorrow).
Comment 9 Carl Worth 2005-09-13 10:22:03 UTC
Created attachment 3253 [details] [review]
HEAD patch 1/2: Fix locking in cairo_scaled_font_reference/destroy

Here's a patch that I think is ready to commit. It's the same basic thing as
before, but split into two parts with better documentation, and more testing.

I haven't been able to replicate the apparent deadlock I mentioned before.
I may have just been testing against the wrong installed version of cairo.
Comment 10 Carl Worth 2005-09-13 10:23:37 UTC
Created attachment 3254 [details] [review]
HEAD patch 2/2: Add missing locking to _cairo_scaled_glyph_lookup

Here is part 2 of 2.
Comment 11 Carl Worth 2005-09-13 15:19:17 UTC
I've now committed these two patches to the HEAD of CVS (ChangeLog below),
so the fix is in place for 1.2.0 now.

As for 1.0.0, the patches don't apply directly. The first needs to modify
cairo-font.c rather than cairo-scaled-font.c. (I've committed this in the
BRANCH_1_0 branch so that fix is in place for 1.0.2). The second patch is not
relevant to 1.0.0 at all, (the bug is in new code added since then).

I've added the test case to "make check" as test/pthread-text.c on both branches.

There's still one remaining, similar bug that exists in BRANCH_1_0. There
the test case triggers the following:

    lt-pthread-show-text: cairo-ft-font.c:677:
    _cairo_ft_unscaled_font_set_scale: Assertion `error == 0' failed.

I'm looking into that now.

-Carl

2005-09-13  Carl Worth  <cworth@cworth.org>

        * src/cairo-scaled-font.c: (_cairo_scaled_glyph_lookup): Add
        missing locking around the call into the thread-shared cache here.

2005-09-13  Carl Worth  <cworth@cworth.org>

        * test/pthread-show-text.c: (start), (main): Adjust parameters to
        stress things a bit more, (better exposing another locking bug).

2005-09-13  Carl Worth  <cworth@cworth.org>

        * src/cairo-scaled-font.c: (cairo_scaled_font_reference),
        (cairo_scaled_font_destroy): Expand locking to encapsulate any
        modification to the reference count of a scaled font, rather than
        just modifcations of the scaled_font_map, since scaled fonts are
        shared between threads.

2005-09-13  Carl Worth  <cworth@cworth.org>

        * test/cairo-test.h: Add documentation for cairo_test functions.

        * test/cairo-test.c: (cairo_test_init), (cairo_test_expecting):
        Abstract log fie creation into cairo_test_init for use by tests
        that don't use cairo_test().

        * test/.cvsignore:
        * test/Makefile.am:
        * test/pthread-show-text.c: (start), (main): Add new test for bug
        #4299 as reported by Alexey Shabalin.
Comment 12 Carl Worth 2005-09-13 15:39:52 UTC
Created attachment 3260 [details] [review]
BRANCH_1_0 patch 1/2: Fix locking in cairo_scaled_font_reference/destroy
Comment 13 Carl Worth 2005-09-13 15:42:32 UTC
Created attachment 3261 [details] [review]
BRANCH_1_0 patch 2/2: Fix locking in _cairo_ft_scaled_font_text_to_glyphs

Here are two patches that should apply cleanly to the 1.0.0 source and should
fix
the bugs.

These have both now been applied to the BRANCH_1_0 branch in CVS.

I'm marking this as fixed, but I would appreciate your verfiying that the fixes

work, (either by applying the patches here, or by checking out the HEAD or the
BRANCH_1_0 revision from CVS). Thanks.
Comment 14 Carl Worth 2005-09-13 15:43:39 UTC
Necessary fixes now committed to the BRANCH_1_0 branch as well.

Marking this bug resolved as fixed.

2005-09-13  Carl Worth  <cworth@cworth.org>

        * src/cairo-ft-font.c: (_cairo_ft_scaled_font_text_to_glyphs):
        Expand locking to include all modification of cairo_scaled_font
        and related objects.

2005-09-13  Carl Worth  <cworth@cworth.org>

        * test/pthread-show-text.c: (start), (main): Adjust parameters to
        stress things a bit more, (better exposing another locking bug).

2005-09-13  Carl Worth  <cworth@cworth.org>

        * src/cairo-scaled-font.c: (cairo_scaled_font_reference),
        (cairo_scaled_font_destroy): Expand locking to encapsulate any
        modification to the reference count of a scaled font, rather than
        just modifcations of the scaled_font_map, since scaled fonts are
        shared between threads.
Comment 15 Alexey Shabalin 2005-09-14 00:39:09 UTC
The simpler test you have attached before is going fine with your code changes.
But my tests (I have attached them earlier) are failing pretty soon with the 
exactly same error "cairo_scaled_font_refernce : Assertion 'i< font_map-
>num_holdovers' failed. Aborted "
I used  the same library for your and my test.  
The difference is in that I'm using "svg_cairo_render" in my test instead of 
cairo's drawing procedures how your test does.
So I'm afraid the bug is persist.
Comment 16 Carl Worth 2005-09-14 11:36:40 UTC
Sorry, I should have retested against your original test (CairoTest.cpp). I've
done that now. The test has a bug in the handling of isActive, (modified by
multiple threads), which often makes the test exit before it gets started. I've
worked around this with the following simple patch:

--- CairoTest.cpp.orig  2005-09-14 10:55:13.656682400 -0700
+++ CairoTest.cpp       2005-09-14 11:05:47.531318808 -0700
@@ -323,7 +323,7 @@
             pthread_create(&tid, NULL,MyThreadProc, this);
         }
         // wait until all threads completed
-        while (isActive) {
+        while (true) {
             sleep(10 /*00*/);
         }

I also cleaned up the Makefile to remove an errant tab, use pkg-config to find
libraries rather than assume they are installed in local directories, and remove
the -pedantic flag which complains about a trailing comma in an enumerator list
in svg.h. Here's the patch for those things:

--- Makefile.orig       2005-09-14 11:06:46.881296240 -0700
+++ Makefile    2005-09-14 10:58:21.669100168 -0700
@@ -3,18 +3,17 @@

 SOURCES := \
        CairoTest.cpp
-

 OBJECTS := $(SOURCES:.cpp=.o)


 CC =/usr/bin/g++
-CFLAGS =   -I ./cairo -ansi
+CFLAGS = `pkg-config --cflags libsvg-cairo`

-LIB_PATH = -L ./libs
+LIB_PATH = `pkg-config --libs libsvg-cairo`
 LIBS =     -lcairo -lsvg-cairo

-DFLAGS = -g -pedantic
+DFLAGS = -g
 #DEFINES = -DLINUX -DGCC_296 -DDEBUG
 DEFINES = -DLINUX

If the test builds and runs fine for you, you shouldn't necessarily need any of
the above. I just wanted to include them for completeness in case we have more
trouble replicating behavior.

With those patches in place, I have made four builds of cairo, and installed
each one to a separate location by means of the --prefix argument to configure,
(for example, "./configure --prefix=/opt/cairo-head").

1) cairo-head: a build from "cvs checkout" as of now, (meaning it includes the
committed HEAD patches attached above).

2) cairo-1.0.1: a build from "cvs checkout -r BRANCH_1_0" as of now, (meaning it
includes the committed BRANCH_1_0 patches attached above).

3) cairo-1.0.0: a build from the tar file:
http://cairographics.org/releases/cairo-1.0.0.tar.gz

4) cairo-1.0.0-with-4299-patches: same tar file as (3) but with the two
BRANCH_1_0 patches applied manually first, (one reject in the ChangeLog which I
ignored).

I'm running the test by setting LD_LIBRARY_PATH to the appropriate directories
in turn. For example:

LD_LIBRARY_PATH=/opt/cairo-head/lib:${LD_LIBRARY_PATH} ./MyTest map_asu.svg 30

I've performed several runs of this test as above, changing only the initial
directory provided to LD_LIBRARY_PATH. The results I see are as follows:

cairo-1.0.0
-----------
Program crashes with "Assertion `i < font_map->num_holdovers' failed." every
time, (after many "Started thread" messages print but before any "Conversion
completed" messages).

cairo-1.0.0-with-4299-patches, cairo-1.0.1, cairo-head
------------------------------------------------------
No crashes seen in any attempts. Program always runs, continuously printing
"Conversion completed" messages until I get bored and terminate it, (I've run it
against each version for at least about two minutes).

Can you test again, and if you are still finding problems try to identify what
is different between our test procedures?

Thanks,

-Carl

Comment 17 Alexey Shabalin 2005-09-14 23:57:58 UTC
I have retested again. All works fine now, without assertion's violation.
I think my problem was in wrong paths to libraries. I used old library instead 
the new built one when running my test.

Thank you for support, you really helped me in my work. I hope new corrected 
release will be available soon.

With best regards,
Alexey Shabalin
Comment 18 Alexey Shabalin 2005-09-15 00:13:49 UTC
Oo-ps!
I have tested it during ten minutes and it was working fine. I moved status of 
bug to "fixed",..
but..
But after that I got the same assertion violation message again and test was 
aborted! :-(

What may be wrong again, after many correct cycles?
Comment 19 Alexey Shabalin 2005-09-15 00:16:26 UTC
Created attachment 3274 [details]
Corrected makefile

Changes were made according to your advice
Comment 20 Alexey Shabalin 2005-09-15 00:17:16 UTC
Created attachment 3275 [details]
Corrected cpp test file
Comment 21 Carl Worth 2005-10-03 15:16:01 UTC
If we're going to make progress on this bug report, you'll need to provide more
specifics about how you have replicated the failure. I have not succeeded in
replicating the bug since the patches have been applied.

See my comment #16 in which I specified exactly which versions of cairo with
which patches I ran against, what test case I ran, how it failed and how often.

Could you please supply similarly detailed information on where you are seeing
problems so that I can replicate them?

Thank you.
Comment 22 Alexey Shabalin 2005-10-03 23:31:04 UTC
It was very easy to repeat the crash. You told in your previous report what all 
is going fine if you ran the tests during "at least two minutes each".  Because 
I need the Cairo as in component of service that should stably work without 
failures during several months I'm forced to run the tests during more continued 
time at least several hours. How I already told the test has been failed after 
about fifteen minutes. It failed with the same error message "Assertion `i < 
font_map->num_holdovers' failed."
What about details:
I was using "cairo-1.0.0: a build from the tar file", had applied the two 
"BRANCH_1_0" patches at first. I have not tried your steps 1) and 2) because I'm 
not using the CVS. 
As test I was using my "conversion application".  For my test app I also used 
libraries libsvg with version 0.1.4 and libsvg-cairo, version 0.1.6.
Comment 23 Alexey Shabalin 2005-12-02 19:51:54 UTC
Carl,
I just want to remind you that the bug is still alive. 
I ran my multithread test that converts SVG file into PNG in loop cycle. 
And it have crashed with the same error message after several minutes of 
successful running.
I used the latest releases of Cairo's libraries: cairo-1-0-2, libsvg-0.1.4, 
libsvg-cairo-0.1.6
Comment 24 Behdad Esfahbod 2006-11-20 13:54:31 UTC
Alexey, can you see if the patch in bug 8801 fixes your crashes?
Comment 25 Alexey Shabalin 2006-11-24 01:54:57 UTC
(In reply to comment #24)
> Alexey, can you see if the patch in bug 8801 fixes your crashes?

Unfortunately it doesn't.
I tested on cairo-1.2.4,libsvg-0.1.4, libsvg-cairo-0.1.6
The test application crashes practically at once. More sooner than before.
I'm going to test on the latest Cairo release (or snapshot) and then let you 
know.
Comment 26 Alexey Shabalin 2006-11-24 04:14:36 UTC
(In reply to comment #25)
> (In reply to comment #24)
> > Alexey, can you see if the patch in bug 8801 fixes your crashes?
> 
> Unfortunately it doesn't.
> I tested on cairo-1.2.4,libsvg-0.1.4, libsvg-cairo-0.1.6
> The test application crashes practically at once. More sooner than before.
> I'm going to test on the latest Cairo release (or snapshot) and then let you 
> know.
> ---------
NEW:

I tried to test the application on cairo-1.2.6 and cairo-1.3.4 with the same 
libsvg and libsvg-cairo with and without these patches.

The test application files at once when number of threads >1.

I got the following messages instead previous one: 

Sometimes this:

"cairo-surface.c:416: cairo_surface_reference: Assertion `surface->ref_count > 
0' failed."

Or sometimes:

cairo.c:87: _cairo_error: Assertion `status > CAIRO_STATUS_SUCCESS && status <= 
CAIRO_STATUS_INVALID_INDEX' failed.

cairo-ft-font.c:562: _cairo_ft_unscaled_font_unlock_face: Assertion `unscaled-
>lock > 0' failed.

cairo-cache.c:213: _cairo_cache_thaw: Assertion `cache->freeze_count > 0' 
failed.

cairo-ft-font.c:562: _cairo_ft_unscaled_font_unlock_face: Assertion `unscaled-
>lock > 0' failed.

Comment 27 Jan Słupski 2007-01-22 18:33:17 UTC
(In reply to comment #25)
> (In reply to comment #24)
> > Alexey, can you see if the patch in bug 8801 fixes your crashes?

Heh, again...
Alexey, can you see if the patch in bug 8801 fixes your crashes?
Comment 28 Inocencio Richiez 2007-06-25 11:41:15 UTC
I'm also having an issue with this. I just recently upgraded/updated my RedHat 4 Linux and started getting: 
cairo-ft-font.c:677: _cairo_ft_unscaled_font_set_scale: Assertion `error == 0' failed.
When I launch my app. Is there a workaround this? I've never have apply a patch to Linux before. I looked it up and they warn people about the kernel when doing this. Can someone help???
Comment 29 Behdad Esfahbod 2007-06-25 11:45:14 UTC
This is most probably fixed in 1.4.8 already.
Comment 30 Inocencio Richiez 2007-06-25 11:47:11 UTC
FYI:
# cat /proc/version
Linux version 2.6.9-55.ELsmp (brewbuilder@ls20-bc2-14.build.redhat.com) (gcc version 3.4.6 20060404 (Red Hat 3.4.6-3)) #1 SMP Fri Apr 20 17:03:35 EDT 2007
Comment 31 Inocencio Richiez 2007-06-25 11:49:14 UTC
How do I upgrade to 1.4.8?
Comment 32 Inocencio Richiez 2007-06-25 12:36:57 UTC
With latest updates to RedHat 4 as I indicated above, I still get this problem with the:
cairo-ft-font.c:677: _cairo_ft_unscaled_font_set_scale: Assertion `error == 0'
failed.
Comment 33 Inocencio Richiez 2007-06-25 12:40:12 UTC
# cat /etc/redhat-release
Red Hat Enterprise Linux ES release 4 (Nahant Update 5)
Comment 34 Chris Wilson 2008-10-10 08:09:29 UTC
Hmm, multiple threadsafety issues reported - all I believe addressed. Please reopen, if this issue has not been fully resolved.


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.