[Bug 40283] Crash when a client needs recovery

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=40283

--- Comment #5 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2011-08-23 05:00:42 PDT ---
(In reply to comment #4)
> MC maps each channel to its handler using the well-known-name of the handler
> process. In this case, MC needs to know who's handling each channel to check if
> it bypass observers so it knows when an observer recovers for each channel if
> it should be given to it or if the handler wants to bypass them.

Fair enough... but in that case, it should only be looking for a Handler head
on that process, and preferably one whose filter matches the channel.
_mcd_dispatcher_reinvoke_handler has similar logic for EnsureChannels (although
the behaviour in corner cases is different - if EnsureChannels discovers that
there is no matching handler, it just gives up).

I'm not sure how much BypassObservers=True makes sense on a client that will
Claim() channels: for new channels, the observer-bypassing bit is only relevant
if the client has a filter that is known in advance.

Perhaps the check we actually want is something like this:

* if we know the Handler well-known-name &&
  the corresponding Handler still exists

  * return its BypassObservers property

* else

  * foreach Handler with the given well-known name

    * if it has BypassObservers = TRUE, return TRUE

  * else, return FALSE

Or even just:

* get the possible handlers for the channel, as if it was a new channel
  (_mcd_client_registry_list_possible_handlers) - perhaps matching the
  unique name of the Approver that claimed it, or perhaps just all the
  handlers

* if *any of them* has BypassObservers = TRUE, return TRUE

* else return FALSE

It seems BypassObservers is still a draft property (Handler.FUTURE), and the
rationale given in telepathy-spec talks about "use-cases where the handler
doesn't want anyone observing the channel - for example, because channels it
handles shouldn't be logged", so we probably want to err on the side of "if
*anyone* says the channel bypasses observers, then it bypasses observers".

I noticed in passing that
_mcd_dispatch_operation_handlers_can_bypass_observers() is implemented wrong;
I'll open a bug for that.

> I don't know if there are other cases where that mapping is used.

_mcd_dispatcher_reinvoke_handler was the motivation for it. git grep would tell
you whether there are others.

> But I think
> either that mapping is useless and should be removed, or we should make our
> best to keep it complete and correct. We should not inserting NULL as the
> handler of a channel if we can know who's handling it.

At the same time, we perhaps shouldn't be inserting a non-NULL handler for a
channel if it's just a guess anyway - we can guess at least as well, and maybe
even better, at time-of-use (as _mcd_dispatcher_reinvoke_handler does).

(Analogously: git performs rename-detection when you do git log, not when you
do git commit.)

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