| Summary: | Assertion fails in "cairo-font.c" when using multithreads | ||
|---|---|---|---|
| Product: | cairo | Reporter: | Alexey Shabalin <Alexey.Shabalin> |
| Component: | general | Assignee: | Carl Worth <cworth> |
| Status: | RESOLVED FIXED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | normal | ||
| Priority: | high | ||
| Version: | 1.0.0 | ||
| Hardware: | x86 (IA32) | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
Input SVG-file for this test application
My C++ test application to demonstrate the bug Makefile for build this test Simpler test case for bug Initial proposed patch HEAD patch 1/2: Fix locking in cairo_scaled_font_reference/destroy HEAD patch 2/2: Add missing locking to _cairo_scaled_glyph_lookup BRANCH_1_0 patch 1/2: Fix locking in cairo_scaled_font_reference/destroy BRANCH_1_0 patch 2/2: Fix locking in _cairo_ft_scaled_font_text_to_glyphs Corrected makefile Corrected cpp test file |
||
|
Description
Alexey Shabalin
2005-08-30 01:34:18 UTC
*** Bug 4300 has been marked as a duplicate of this bug. *** Created attachment 3146 [details]
Input SVG-file for this test application
Created attachment 3147 [details]
My C++ test application to demonstrate the bug
Created attachment 3148 [details]
Makefile for build this test
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.
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.)
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. 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). 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. Created attachment 3254 [details] [review] HEAD patch 2/2: Add missing locking to _cairo_scaled_glyph_lookup Here is part 2 of 2. 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.
Created attachment 3260 [details] [review] BRANCH_1_0 patch 1/2: Fix locking in cairo_scaled_font_reference/destroy 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. 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. 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.
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
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 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? Created attachment 3274 [details]
Corrected makefile
Changes were made according to your advice
Created attachment 3275 [details]
Corrected cpp test file
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. 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. 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 Alexey, can you see if the patch in bug 8801 fixes your crashes? (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. (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. (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? 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??? This is most probably fixed in 1.4.8 already. 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 How do I upgrade to 1.4.8? 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. # cat /etc/redhat-release Red Hat Enterprise Linux ES release 4 (Nahant Update 5) 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.