[Telepathy] tq-qt4 refcount proposal

[Andre Moreira Magalhaes]

Hi all,

I would like to make a proposal for tp-qt4 refcounted classes.

1 - Rationale:
Many objects (read all high-level DBus interfaces classes AccoutManager, 
Account, Channel, Connection, ...) in tp-qt4 are reference counted.
The pattern used is that classes inherit QSharedData and are used 
together with QExplicitlySharedDataPointer, that takes care of managing 
the reference count.
When the reference count hits 0, the object is deleted.
When instantiating new classes that inherits QSharedData without using 
QExplicitlySharedDataPointer the reference count is 0, this is referred 
to as the floating state.
This may lead to memory leaks, caused by objects in the floating state 
that never get deleted.

2 - Proposal for avoiding objects in floating state as hard as possible:
New objects should be put into a QExplicitlySharedDataPointer as soon as 
possible after creation to avoid possibly leaking an object while it’s 
in a floating state.
To achieve this we could make the objects constructor protected* and 
have public create function that returns a QExplicitlySharedDataPointer.

Why not make the constructor private?
We need to support subclasses, and so we need the constructors to be at 
least protected.

2.1 - Problems:
2.1.1 - Circular dependency:
In the ideal world we should use QExplicitlySharedDataPointer<Object> in 
the library, even for objects that needs to know about each other, for 
example StreamedMediaChannel that caches the MediaStreams and 
MediaStreams that have a pointer to the StreamedMediaChannel. We should 
be careful with this, as we hit circular dependency, which can lead to 
even more memory leaks when the objects refcount never hits 0.
This can be handled in different ways, and should be done case by case. 
Bellow there is a code snippet to illustrate how to manage this on 
StreamedMediaChannel/MediaStream case:

class MediaStream : public QSharedData, ... {
public:
MediaStream(const QExplicitlySharedDataPointer<StreamedMediaChannel> 
&channel, ...)
: QSharedData(),
mChannel(channel) // channel->ref++
{ }

...

QExplicitlySharedDataPointer<StreamedMediaChannel> channel() const { 
return mChannel; }

...

private:
...

QExplicitlySharedDataPointer<StreamedMediaChannel> mChannel;
};

class StreamedMediaChannel : public QSharedData, ... {
public:
static QExplicitlySharedDataPointer<StreamedMediaChannel> create(...) { 
... }

...

QList<QExplicitlySharedDataPointer<StreamedMediaChannel> > streams() 
const { return mStreams; }

...

protected:
StreamedMediaChannel(...); // StreamedMediaChannel should allow inheritance

...

private Q_SLOTS:
void onInvalidated(...) // closed
{
foreach (stream, streams) {
// if nothing else is holding the stream, the stream will be deleted
stream.reset();
}
// no stream is referencing the channel anymore, when the library user 
forget about the channel,
// it will be deleted.
}

void onStreamRemoved(...)
{
...
// if nothing else is holding the stream, the stream will be deleted
stream.reset();
...
}

private:
...

QList<QExplicitlySharedDataPointer<MediaStream> > mStreams;
};


2.1.2 - Custom classes:
All custom classes should follow this guideline, making the constructor 
protected and having a create method, to avoid objects in floating 
state, as explained above.

2.1.3 - QObject parent relationship:
All refcounted objects should not have a parent QObject, as when the 
QObject gets deleted the children objects are deleted as well.

3 - Naming:
As you can see typing QExplicitlySharedDataPointer all over the place is 
not something really nice, and we may want to move to another class 
later, a lightweight version or something else, so the ideal is to have 
our own class that for now inherits QExplicitlySharedDataPointer, and if 
we want to change it later we just change this class.
Note that typedef is not an option here, as you can't typedef template 
(just template specializations) and can't forward declare typedef, which 
we would need.

So I propose to have a template class SharedPtr that inherits 
QExplicitlySharedDataPointer, and a class named SharedData that is a 
typedef for QSharedData (in case we want to change to use another class 
in the future, we don't need to change all classes). We may want to make 
SharedData call deleteLater on ref==0 instead of delete for example.

So the usage would be:

public Account : SharedData
{
public:
...
static SharedPtr<Account> create();
...
};

I wrote a experimental branch to ilustrate what it's explained above
http://git.collabora.co.uk/?p=user/andrunko/telepathy-qt4.git;a=shortlog;h=refs/heads/shared

Opinions, suggestions?

BR
Andrunko

P.s.: Some ideas are based on http://webkit.org/coding/RefPtr.html, as 
suggested by Kenneth Rohde Christiansen.