[PATCH 0/9] Add references counter to GrlMediaSourceFooSpec structures
- From: "Juan A. Suarez Romero" <jasuarez igalia com>
- To: grilo-list gnome org
- Subject: [PATCH 0/9] Add references counter to GrlMediaSourceFooSpec structures
- Date: Fri, 9 Jul 2010 18:53:57 +0200
As you know, when implementing browse(), search() or query() functions in
sources, you must be aware that when invoking the callback with remaining == 0,
the spec containing data is destroyed, so you can't access its contents.
This led to some bugs some time ago, as sources were using this spec even after
the callback (for instance, it was quite common to invoke the callback and
aftewards, check if spec->count was 0 or not). This was fixed by copying
required data in a different place.
Well, actuallym, from my point of view, this behaviour is a bit dissapointed.
It would be better to destroy the spec *after* source operation returns, and
not inmediately after invoking the callback.
So inspired by how GHashTable works, I added a refcount mechanism in
GrlMediaSourceFooSpec structures. Thus, instead of directly destroying the
struture, it is unrefered. And when creating it, it is done with refcount to 1.
So when source invokes callback(remaining==0), spec is unrefered.
The trick is that core invokes ref() with the spec just before invoking source
operation, and unref() it just after returning. If you think it carefully, this
achieves the effect we wanted: the spec is destroyed after source operation
returns, not after invoking the callback, so source can still use it.
Moreover, if the source, for any reason, requires to keep the spec for any
reason (one of the main reasons could be that this spec is wrapped in another
structure, which is handled in an idle function even after sending the last
element), it can ref() the structure so it is not destroyed. Obviously, it
would need to invoke the corresponding unref() to free it when it is not
needed. Not needed to say that sources should never ever invoke an unref()
without the corresponding previously ref().
As a plus, the last two patches add setters and getters to
GrlMediaSourceFooSpec. Right now, I only changed core to use the setters, but
But I would like to ask you what do you think if we seal the spec fields,
making them private, and make remaining core and plugins to use the appropriate
get/set functions. For you information, GHashTable do it this way.
Any comment is welcome.
Juan A. Suarez Romero (9):
core: Add references counter to GrlMediaSourceFooSpec structures
core: Add constructors to GrlMediaSourceFooSpec
core: Use constructors to create GrlMediaSourceFooSpec
core: Add ref() functions to GrlMediaSourceFooSpec
core: Add unref() functions to GrlMediaSourceFooSpec
core: Do not directly destroy GrlMediaSourceFooSpec structures
core: Protect browse/search/query specs before invoking source
core: Add setters and getters to GrlMediaSourceFooSpec
core: Use setters functions to access GrlMediaSourceFooSpec fields
src/grl-media-source.c | 419 +++++++++++++++++++++++++++++++++++-------------
src/grl-media-source.h | 146 +++++++++++++++++
2 files changed, 457 insertions(+), 108 deletions(-)
] [Thread Prev