| Summary: | Hash table contains zero referenced font faces! | ||
|---|---|---|---|
| Product: | cairo | Reporter: | Maximilian Heinzler <maxi> |
| Component: | win32 backend | Assignee: | Maximilian Heinzler <maxi> |
| Status: | RESOLVED FIXED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | critical | ||
| Priority: | medium | ||
| Version: | 1.10.3 | ||
| Hardware: | All | ||
| OS: | Windows (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
Simple example that shows the bug under windows.
Implementation of win32 font destruction |
||
(In reply to comment #0) > After two hours of debugging and searching I found the source of this bug. Which bug? I get a font-related crash in the testsuite on win32, but it is not fixed by your patch, so I assume you are seeing some other crash. > The win32 backend does not remove font faces from its hash table it they are > about to be destroyed. > > I think this code will fix it: > static void > _cairo_win32_font_face_destroy (void *abstract_face) > { > cairo_win32_font_face_t *font_face = abstract_face; > > _cairo_hash_table_remove (cairo_win32_font_face_hash_table, > &font_face->base.hash_entry); > } > > I don't know for sure if there also has to be an check for > "font_face->base.hash_entry.hash != 0" like the toy-font backend has it. I think that you should not be accessing cairo_win32_font_face_hash_table directly, but using _lock()/_unlock(). Created attachment 47726 [details]
Simple example that shows the bug under windows.
I've added an file that shows the bug i'm talking about. It also includes the whole patch.
Maybe the test crashes because there is a second thing i have forgotten. In the "_cairo_win32_font_face_hash_table_destroy" there is an argument missing to "_cairo_hash_table_random_entry". The attachment includes this.
This is the first time i'm using git but i will try myself to upload this patch and other patches.
Created attachment 47754 [details] [review] Implementation of win32 font destruction (In reply to comment #2) > Created an attachment (id=47726) [details] > Simple example that shows the bug under windows. > > I've added an file that shows the bug i'm talking about. It also includes the > whole patch. I can confirm that your program triggers the bug and your patch fixes it. > > Maybe the test crashes because there is a second thing i have forgotten. In the > "_cairo_win32_font_face_hash_table_destroy" there is an argument missing to > "_cairo_hash_table_random_entry". The attachment includes this. > > This is the first time i'm using git but i will try myself to upload this patch > and other patches. I tried to improve your patch a little (see the new attachment). Some investigation of the magic 0 hash might be needed before merging (and I'm probably going to make your program a cairo testcase so that it can be easily run along with the other regression tests). Sorry, I was wrong about being able to reproduce this bug. I'm still seeing another bug (which might be also interacting with this one) which seems to be in toy fonts on win32. Having cairo/master working on win32 would be a good start to easily reproduce/fix bugs :( Could you please tell me which file or code leads to the bug your are talking about? (In reply to comment #6) > Could you please tell me which file or code leads to the bug your are talking > about? In the last few days I pushed some commits to fix the win32 build, now it should be easier to reproduce the problem. I also pushed a branch containing some more win32 (and/or font) related fixes here: http://cgit.freedesktop.org/~ranma42/cairo/log/?h=wip/win32-font Does your program hit the assert if linked to this branch? No, it works fine. Thank you! It should be fixed in master by: commit 94bc20da50c5984e5c04929a7fde4c2f04e66380 Author: Andrea Canciani <ranma42@gmail.com> Date: Fri Jun 17 19:15:44 2011 +0200 win32-font: Implement destroy function Win32 font faces can be removed from the hashtable upon destruction. Based on the toy-font destruction code. See https://bugs.freedesktop.org/show_bug.cgi?id=38049 and commit 101fab7cd8a90f7cf3d8113c792b3f8c2a9afb7d Author: Andrea Canciani <ranma42@gmail.com> Date: Wed Jun 15 11:37:36 2011 +0200 win32-font: Improve static data reset function The hashtable is guaranteed to only contain font faces which are currently referenced, hence there is no need to remove any font face when it is reset (just like for toy-font). This makes the function simpler and fixes the assertion Assertion failed: predicate != NULL, file cairo-hash.c, line 373 hit by multiple tests (the first one being "clear"). See https://bugs.freedesktop.org/show_bug.cgi?id=38049 |
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.
After two hours of debugging and searching I found the source of this bug. The win32 backend does not remove font faces from its hash table it they are about to be destroyed. I think this code will fix it: static void _cairo_win32_font_face_destroy (void *abstract_face) { cairo_win32_font_face_t *font_face = abstract_face; _cairo_hash_table_remove (cairo_win32_font_face_hash_table, &font_face->base.hash_entry); } I don't know for sure if there also has to be an check for "font_face->base.hash_entry.hash != 0" like the toy-font backend has it.