[Bug 23155] Connection: make handles survive until disconnection

[bugzilla-daemon at freedesktop.org]

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

--- Comment #5 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-11-09 09:59:10 PST ---
(In reply to comment #1)
> TpIntSet should be reimplemented to deal with sparse sets better, ideally
> O(number of members) rather than O(largest member).

I did that.

(In reply to comment #4)
> I'm not comfortable with the idea of tying resource management functionality
> (handle lifetime management) with user-visible behavior (connection getting
> disconnected). True, it makes everything a lot simpler and in typical usage
> accumulation of unused handles is not expected to be a big problem. But still,
> on long-lived connections immortal handles basically constitute a specified
> memory leak, potentially bringing heap fragmentation and whatnot.

This is becoming a pressing concern, because Guillaume is working on Bug
#29531, and I don't want to have the same madness in telepathy-glib as in the
corresponding bit of telepathy-qt4.

I discussed this with Sjoerd this morning and we're still of the opinion that
"leaking" handles is only a theoretical concern. In practice, you communicate
with a finite number of contacts; even the pathological example of an XMPP
version of #ubuntu is "only" 150K or so, and while you're in the room, the CM
needs to hold all of those in memory *anyway* (most likely twice - once for
itself, and once in its underlying library).

One implementation point we need to keep in mind is that APIs resulting in an
unbounded number of handles should be avoided. We preemptively made
ContactSearch operate in terms of identifiers for this reason, so that's OK.

> If letting the clients control the lifetime of handles is too painful, maybe
> the right to garbage-collect handles, possibly after a suitably long time
> period, should be given to the connection managers?

No, I think specifying a "leak" is less bad than specifying behaviour that, in
practice, clients can never recover from.

> Caution, brainstorm-in-progress follows: in a future backwards-incompatible
> Telepathy version, we could have a signal telling the clients that such and
> such handles were invalidated. Any client binding that wants to retain handles
> which it does not hold already should asynchronously reference or hold them,
> and handle or expose the case of concurrent invalidation, which should never
> happen in practice if CMs invalidate handles long time after last usage.

I don't see how the binding or UI could deal with concurrent invalidation
gracefully, and I suspect the machinery to track last-use is "heavier" than
just keeping a grow-only string pool.

To get a sane client API without re-ordering, I think we need to be able to
make a TpContact or Tp::Contact from a (handle, identifier) pair, synchronously
(if it's not synchronous, we re-order events). Given that, if we allow deferred
handle-referencing, we get this situation:

* for maximum badness, assume the client is an Observer, so you have no control
over when messages are acknowledged
* receive a Messages message from Sjoerd, handle 42, identifier sjoerd at night
* client calls _tp_contact_new_sync (conn, 42, "sjoerd at night") -> 0xf00baa,
attaches it to the TpSignalledMessage object and gets on with its life [A]
* client calls either HoldHandles([42]) or RequestHandles(["sjoerd at night"]) in
the background [B]
* (time passes, your dbus-daemon is stalled for some reason; perhaps someone's
using Tracker ;-)
* handle 42 is deallocated [C]
* handle 42 is reallocated to point to "smcv at reptile"
* the CM replies to B
* the client gets the reply [D]

With your proposal, the client gets a signal either at C or after D;
regardless, the client gets a reply at D. In either case, it says that the
state of the object 0xf00baa is inconsistent with reality. What's the client
going to do about it? Between A and either C or D, it's been taking actions
based on the information it thought it had; it can't necessarily undo those
actions.

Having the CM do delayed-deallocation just increases the length of time for
which you need to have been not scheduled to get this problem; it doesn't make
it any more possible to recover.

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