Re: [Tracker] Refactored the src/plugins & (new) Evolution support

[Philip Van Hoof <spam pvanhoof be>]

On Thu, 2009-02-12 at 10:15 +0000, Martyn Russell wrote:
> Philip Van Hoof wrote:
> > On Wed, 2009-02-11 at 17:55 +0000, Martyn Russell wrote
 
> > *-indexer-module.la is an 'indexer' module which runs in
> > tracker-indexer. It will pump the data actually into the database.
> >
> > *-daemon-module.la is a 'deamon' module which runs in trackerd. It will
> > receive the data from the external application (presumably over DBus)
> > and it will 'pass it on' to the *-indexer-module.la.
>
> The first thing that I think here is, why do we need both? Especially if 
> we are going to be moving the crawling and queueing all in to the 
> indexer anyway (at a later date). Is it just so we have a permanent 
> entity (trackerd) always available to receive data? Is there some other 
> reason?

The -daemon-module.la is to have a permanent entity to receive data,
indeed. This was already the case for the existing new Evolution code.

This is just a modularized version of the same infrastructure (so that
you can more easily repeat it for multiple such data providing
use-cases).

> > > #13, For the cases where we reference another object here, we need to 
> > > make sure we reference the new object first. We do this to protect 
> > > ourselves from unreferencing the same object we are about to reference, 
> > > (in the unlikely event that we set the object to the same object we 
> > > already have in priv->object):
> > >
> > > +void
> > > +tracker_push_registrar_set_object (TrackerPushRegistrar *registrar,
> > > +                             GObject              *object)
> > > +{
> > > +  TrackerPushRegistrarPrivate *priv;
> > > +
> > > +  priv = TRACKER_PUSH_REGISTRAR_GET_PRIVATE (registrar);
> > > +
> > > +  if (priv->object)
> > > +          g_object_unref (priv->object);
> > > +
> > > +  if (object)
> > > +          priv->object = g_object_ref (object);
> > > +  else
> > > +          priv->object = NULL;
> > > +}
> > > +
> >
> > So, what is the problem here exactly?
>
> Say priv->object = 0xdeadbeef. Say object (passed in as an argument to 
> the function) is also 0xdeadbeef, both are the same object. The object 
> has 1 reference. We then remove that reference (which finalizes it) and 
> then we try to ref it again. It is better in these conditions to always 
> ref the new object just in case we are silly enough somewhere to set the 
> object to the same thing we already have set.

Allowing NULL is being used to reset the object in various places in the
code. It would make it complicated if I wouldn't deal with NULL.

For example these two in tracker_evolution_push_registrar_enable:

tracker_push_registrar_set_object (registrar, NULL);
tracker_push_registrar_set_manager (registrar, NULL);

The reason is that NameOwnerChanged happens in irregular ways for a new
or a being-deleted service. It for example sometimes happens twice per
service, in an inconsistent way.

Regretfully there's no heuristic that I can apply here. Whether you get
NameOwnerChanged twice, three times or just once seems to depend on the
moon, Ubuntu version, Fedora version, DBus version, amount of other
services, Richard Dawkins's thee-pot between Mars and Earth, etc

So each time it happens I need to 'destroy' any existing DBus object
being registered for the path, and create a new one. Not just unref a
DBus object, but instead being sure that it's destroyed instead. 

That's the reason for the resetting (and using NULL is the method to be
used for that resetting)

Note that the reference count for that reason can't ever be > 1, unless
you use the instances wrong (of course). So setting to NULL always
destroys (unless you are using the instances wrong, but that's a bug
that must just be fixed then).


-- 
Philip Van Hoof, freelance software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://pvanhoof.be/blog
http://codeminded.be