[Bug 23155] Connection: make handles survive until disconnection

[bugzilla-daemon at freedesktop.org]

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

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Connection: refcount        |Connection: make handles
                   |handles                     |survive until disconnection
             Status|NEW                         |ASSIGNED
         AssignedTo|telepathy-bugs at lists.freede |simon.mcvittie at collabora.co
                   |sktop.org                   |.uk
          QAContact|                            |telepathy-bugs at lists.freede
                   |                            |sktop.org

--- Comment #1 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-04-05 11:29:36 PDT ---
The current handle model causes a number of unfortunate corner cases. Many
can be solved with something equivalent to MembersChangedDetailed's
contact-ids hint; MessageReceived needs to gain one of these (I'll clone
this bug).

However, we also have the problem that telepathy-glib's TpContact and
telepathy-qt4's Tp::Contact guarantee that they are holding a handle. This
can't be done without a D-Bus round trip (in at least some cases),
leading to problems like:

* Channel receives a message with sender #42, "sjoerd"
* Channel wants to emit a Message signal with a Contact object for sjoerd
* A Contact object for sjoerd is not in the cache
* HoldHandles(CONTACT, [42]) sent
* Channel receives a message with sender #23, "smcv"
* A Contact object for smcv *is* in the cache
* Channel emits message-received for the message from smcv
* Time passes
* CM responds to HoldHandles
* Channel emits message-received for the message from sjoerd
* The messages from sjoerd and smcv have been re-ordered!

telepathy-qt4 goes to heroic lengths to avoid this for Messages and for
Group changes (it has an internal queue, and delays the Qt signals until
it can form a Contact object), but Messages and Group events can still
get re-ordered.

My goal for solving this problem is that there's a (relatively) simple
set of extra things that CM authors (or, in practice, the libraries they
use) can do. If they do, client code interacting with their CM will use
a fast path that always creates contact objects synchronously and never
re-orders; if they fail to do the extra things, their CM is considered
to be deficient, and the client library recovers by using a slow-path
that may re-order messages.

Sjoerd proposes that we just stop ref-counting handles, and have them
all persist until the connection disconnects. Rob and I consider this
to be a good proposal.

Memory cost
===========

A back-of-an-envelope estimate: imagine that I'm in a chatroom as busy
as #ubuntu (about 1500 users), but the chatroom is on an XMPP server
with a long name. hypothetical-room at conference.pseudorandom.co.uk is
47 characters; suppose pessimistically that appending users' nicknames
leads to 100-character JIDs. This would lead to 150,000 bytes of string
data for the string pool, which is significant, but not ridiculous.

(Note that UI and CM will both need to store all of these strings for
the duration of the user's presence in the chatroom in any case - the
only change here is that when the user leaves the chatroom, the strings
aren't freed until the connection disconnects.)

A more problematic point is that TpIntSet is currently optimized for
relatively dense sets of small integers; it's an expanding bitfield with
one bit per handle, up to the largest handle it contains. The silliest
case for someone in #ubuntu is a TpIntSet with one member, handle #1500,
which would be 188 bytes of 0, with one 1 near the end.

Implementation in the spec
==========================

property Connection.HasImmortalHandles: b
    TRUE if handles last for the whole lifetime of the connection. This
    SHOULD be the case in all connection managers, but clients MUST
    interoperate with connection managers that reference-count handles.

method Connection.HoldHandles (u, au) -> nothing
    If Connection.HasImmortalHandles is true, this does nothing and returns
    successfully.

    Older connection managers give it the following semantics:
    ...

method Connection.ReleaseHandles (u, au) -> nothing
    If Connection.HasImmortalHandles is true, this does nothing and returns
    successfully.

    Older connection managers give it the following semantics:
    ...

method Connection.Interface.Contacts.GetContactAttributes(...) -> ...
    ...
    Hold: b
        If Connection.HasImmortalHandles is true, this parameter has no
        effect. On older connection managers, ...

Implementation in telepathy-glib (for CMs)
==========================================

TpIntSet should be reimplemented to deal with sparse sets better, ideally
O(number of members) rather than O(largest member).

TpDynamicHandleRepo could be preserved mostly as it is, with the handle
refcounting, handle-leak tracking, etc. stubbed out, but it would be more
efficient to reimplement it as a simple growing GPtrArray of strings, indexed
by the handle.

Implementation in client libraries
==================================

(I'm talking about telepathy-glib here, but add some :: and it's equally
true for telepathy-qt4...)

TpConnection should get HasImmortalHandles via its GetAll fast-path
(Bug #27459) before becoming prepared (Bug #21097).

TpChannel should wait for the TpConnection to be prepared (which does not
necessarily have to imply connected, any more) before it becomes prepared
itself. On receiving an event (group members change, message, ...),
if there is a TpContact already cached, it's used to signal the event.
Otherwise, if the connection has immortal handles, the TpChannel insta-creates
a TpContact via new internal API, and uses it to signal the event.

As a fallback, the TpChannel makes an async round-trip with the current
public API, and signals the event using the TpContact thus obtained - this
results in the event being delayed (re-ordered), but that's now considered to
be a CM bug (the CM wasn't helpful enough).

We could simplify the fallback code paths by having the client code just leak
all of its handle references, bringing old CMs closer to the new semantics :-)

-- 
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.