[cairo] Fixing concurrency bugs in cairo's reference counting

[Carl Worth]

On Fri, 15 Dec 2006 11:08:43 -0500, "=?UTF-8?Q?Kristian_H=C3=B8gsberg?=" wrote:
> I think a simpler and more consistent (and thus easier to remember)
> rule would be to just always take a ref on behalf of the caller.

The rule might be simpler, but usage would not be. So we've already
discussed and decided against this before the 1.0 release of cairo.

> In the unshared resource case, it requires you to destroy the resource
> later, but the semantics are much cleaner.

That's the inconvenience of usage. It would prevent things like this:

	cairo_surface_create_similar (cairo_get_target (cr), ...);

which turns out to be a commonly desired idiom in GTK+, (which creates
a cairo_t on behalf of the user, without the user first having a
pointer to the target surface).

Under your scheme, this code would have to be written as:

	cairo_surface_t *surface;
	surface = cairo_get_target (cr);
	cairo_surface_create_similar (surface, ...);
	cairo_surface_destroy (surface);

That definitely fails the "easy to use" principle I want for
cairo. And as you mention, it would be an API break anyway, so it's
not even an option.

> Finally, in the shared resource, it's pretty much the only way to go.
> As you say, it requires locking, but what lock is the user supposed to
> take?

Oh, I don't want the user to take any lock here. There's internal
locking, yes, but I think that's all in place in cairo already.

> For example, in the font cache case, some thread creating a new
> font might cause the font you just looked up to disappear.  To prevent
> this, there has to be a lock that font creation and font cache lookup
> shares, and you have to know which entry points in the cairo API can
> evict a font.  This is not feasible.

Huh? The font lookup and eviction code are both using the same cache,
so all that's needed for correct operation is that the cache be locked
even when doing just lookup. And we have that already.

>                                       The only way to deal with this
> is to take a reference on behalf of the caller while holding the lock
> protecting the cache (or other global data structure we're
> traversing).  While holding the data structure lock, we know that all
> objects in the structure have at least one reference owned by the data
> structure, and we can safely add another for the caller.

Yes, that's the behavior I was describing. And we have these cache
locks in cairo already.

>                                                          Once we lift
> the lock, all bets are off and it's too late to ref the looked up
> object.

Yes, that would be too late. My point is that I don't believe there is
anywhere in the cairo API where a user can get an object with no
reference at all. All of the create() functions take a reference on
behalf of the user. The other get() functions do not take a reference,
but I believe only allow a user to get at an object through another
object that already references it.

That is, using the (fairly poor) terminology I used at the beginning
of the thread, the "unshared source" case is exposed in the API, but
the "shared source" case is not. That is, there's no way to ask for
something that's sitting in a cache without any references to it.

So I don't think we have any fundamental problems in the API
here. Does that make sense?

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/cairo/attachments/20061218/8e270475/attachment.pgp