[cairo] Toy font face race condition / heap corruption?

[Paul Messmer]

I'm using Cairo 1.6.4 (though the code at issue seemed basically the same in 1.8.6) in a multi-threaded application and am seeing heap corruption.  The heap corruption went away when stuffed a mutex around the usage of Cairo, so I've been investigating.  It looks like there is a race condition in the management of toy font faces.  The following scenario seems troublesome:

Suppose Thread 1 is using a context with some font (e.g. Helvetica) selected into it.  Suppose it's destroying the context, and cairo_destory calls _cairo_gstate_fini which calls cairo_font_face_destroy with Helvetica.

So Thread 1 enters cairo_font_face_destory (cairo-font-face.c, line 124 in 1.6.4).
1. The reference is positive (1)
2. So _cairo_reference_count_dec_and_test drops the reference count to 0, returning true.
Now let's suspend Thread 1 at this point, before font_face->backend->destroy is executed on line 135.

Now let's have Thread 2 try to use Helvetica in another context for the first time.

Thread 2 enters _cairo_toy_font_face_create (cairo-font-face.c, .line 384).
3.  It locks the hash table, does a lookup on Helvetica, and finds it, since Thread 1 hasn't removed it yet.
4.  It unlocks the hash table, and is about to return &font_face->base on line 408.
Let's suspend Thread 2 as it's about to return from _cairo_toy_font_face_create with the existing object

Back in Thread 1:

5.  font_face->backend->destroy() is called.
6.  This calls _cairo_toy_font_face_destroy which removes the font from the hash table, and calls _cairo_toy_font_face_fini
7.  _cairo_toy_font_face_fini calls "free ((char *) font_face->family)" on line 355.  That seems bad (?) because this object isn't quite dead yet.  Thread 2 is about to return it from _cairo_toy_font_face_create.
8.  _cairo_toy_font_face_fini returns.
9.  _cairo_toy_font_face_destroy returns.
10.  cairo_font_face_destroy (line 141) sees the non-zero reference count and returns without destroying the base part of the font.

Now let's return to Thread 2:

11.  _cairo_toy_font_face_create returns this partially-deleted object.  It's no longer in the hash table so it won't be found again, but presumably at some later point its reference count goes to zero in Thread 2, and there's a double free of memory (possibly now allocated to some new block) when _cairo_toy_font_face_fini calls "free ((char *) font_face->family)" on line 355 again for the orphaned object.


If I add the line "if (CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&font_face->base.ref_count)) return;" after the hash table unlock but before _cairo_toy_font_face_fini in _cairo_toy_font_face_destroy my heap problem seems to go away.


Does anyone have a feel for whether Cairo is well tested in multi-threaded environments?  Or is it just more likely that the toy text API isn't used with any seriousness?

Thanks,
-- Paul
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.cairographics.org/archives/cairo/attachments/20090126/d1ac125d/attachment.html