[Bug 24120] Refactor the McdDispatcher

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24120





--- Comment #20 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2009-10-20 03:50:36 PST ---
(In reply to comment #19)
> I noticed that McdDispatchOperationPrivate has a gchar *claimer. I think that
> should be gchar *plaintiff!

Thanks but no :-P

This is temporary anyway, as fixing Bug #21003 will probably result in
claimer/handler being replaced by a queue of structs representing Claim,
HandleWith and EnsureChannel calls, and the handlers they'd like; when one such
call succeeds, subsequent Claim and HandleWith calls will be told they've
failed.

> +        if (G_LIKELY (dbus_connection))
> +            dbus_g_connection_register_g_object (dbus_connection,
> +                                                 priv->object_path, object);
> 
> Why wouldn't it have one?

Pre-existing, I think. I believe it always would, but you never know in MC...
I'll double-check, and either remove the G_LIKELY or make it some flavour of
assertion.

> I'm a little suspicious about inc_observers() reffing the McdDispatchOperation.
> Surely whatever owns the MDO has its own ref? I fear accidental ref leaks.

At the moment, whatever owns the MDO (the McdDispatcherContext) does have its
own ref. However, it does this by reffing the McdDispatcherContext across
various calls. One of the goals of this branch is to have the
McdDispatcherContext manage its own life-cycle, so the McdDispatcherContext can
be relegated to being a helper struct used to invoke C plugins, which can go
away entirely as soon as the last plugin has finished; this is finally achieved
somewhere around the 4.7 branch.

(The refcount of the McdDispatcherContext is much less amenable to debugging
than that of the MDO, since it's not refdbg'able, leading to strange hacks.)

There is, unfortunately, no signal emitted by the MDO that is suitable to cause
the McdDispatcher to unref it - Finished can perhaps be that signal once I fix
Bug #21003.

> block_finished, may_finish, check_finished, actually_finished? Pretty confusing
> stuff. :)

At the moment Finished is emitted far too early: delaying its emission until we
have *actually* finished is part of Bug #21003. Part of the reason for Finished
being so early is that it tells the Dispatcher to start calling HandleChannels
(until the 6th commit of branch 4.5, which splits it into two signals, one for
D-Bus and one to indicate that dispatching can continue; the signal indicating
that dispatching can continue is later removed, since every effect it caused
was moved to the MDC).

block_finished/unblock_finished are pre-existing; they're deleted halfway
through branch 4.5, in favour of having them be implicit in
inc_observers/dec_observers/inc_ado/dec_ado. may_finish() encapsulates "we are
allowed to emit Finished and ChannelLost if we want to", i.e. "no observers and
no approvers are pending".

finished (renamed to wants_to_finish during this branch, to indicate what it
really means) indicates whether we've reached a situation where Finished should
(in the current implementation) be emitted as soon as possible: i.e. HandleWith
was called, or Claim was called, or EnsureChannel caused approval, or all
channels were lost.

check_finished() is called by anything that decrements the pending
observer/approver count; if there are none pending, it emits any queued
ChannelLost signals, then calls actually_finish() if wants_to_finish is true.

actually_finish() is called by check_finished(), and also whenever
wants_to_finish becomes TRUE while may_finish() is TRUE. It emits any queued
Finished signal. I see your point that actually_finish and check_finished
should probably be one function.

Yes, confusing, but may I file a bug about it instead, and sort it out after
(or more likely, as part of) Bug #21003?


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.