Solid DevicePrivate object ownership

[Jiří Paleček]

Hello,

I've been hunting some memeory leaks in plasma and it seems to me the  
culprit was the solid library. But to understand the matter fully, I need  
clarifications on the following questions:

1) A DevicePrivate for a specific device exists at most once in the  
program, and is always cached in the DeviceManager, is that right?

2) By whom is the m_backendObject of the DevicePrivate owned (and should  
be deleted)?

3)

void Solid::DevicePrivate::setInterface(const DeviceInterface::Type &type,  
DeviceInterface *interface)
{
     ref.ref();
     m_ifaces[type] = interface;
}

Why is the reference count raised every time we add an interface?

4)

void Solid::DevicePrivate::setBackendObject(Ifaces::Device *object)
{
     m_backendObject = object;

     if (m_backendObject) {
         connect(m_backendObject, SIGNAL(destroyed(QObject *)),
                 this, SLOT(_k_destroyed(QObject *)));
     }

     if (!m_ifaces.isEmpty()) {
         foreach (DeviceInterface *iface, m_ifaces) {
             delete iface->d_ptr->backendObject();
             delete iface;
         }

         m_ifaces.clear();

         if (!ref.deref()) deleteLater();
     }
}

And why is it decreased once we reset the backend object?

5)

void Solid::DevicePrivate::setBackendObject(Ifaces::Device *object)
{
...
       foreach (DeviceInterface *iface, m_ifaces) {
             delete iface->d_ptr->backendObject();
             delete iface;
         }

Why do we delete iface->d_ptr->backendObject() here ...

Solid::DevicePrivate::~DevicePrivate()
{
     qDeleteAll(m_ifaces);
}

... and not here?

Regards
     Jiri Palecek

--
Using Opera's revolutionary e-mail client: http://www.opera.com/mail/

Hello,

On Thursday 5 November 2009 11:49:36 Jiří Paleček wrote:
> 1) A DevicePrivate for a specific device exists at most once in the
> program, and is always cached in the DeviceManager, is that right?

Yes, that's the intent.

> 2) By whom is the m_backendObject of the DevicePrivate owned (and should
> be deleted)?

By the DeviceManager (as in some cases the DevicePrivate backend counterpart
should disappear and only the manager knows when that's supposed to happen).

> 3)
>
> void Solid::DevicePrivate::setInterface(const DeviceInterface::Type &type,
> DeviceInterface *interface)
> {
>      ref.ref();
>      m_ifaces[type] = interface;
> }
>
> Why is the reference count raised every time we add an interface?

We want to prevent DevicePrivate to be destroyed even if there's no Device
instance pointing to it as soon as the client code accessed an interface (use
case here is that some interfaces have signals, if the DevicePrivate object
would be destroyed, connecting to those interface signals would be useless as
the emitter would disappear when no corresponding Device instance is left in
the client code).

> 4)
>
> void Solid::DevicePrivate::setBackendObject(Ifaces::Device *object)
> {
>      m_backendObject = object;
>
>      if (m_backendObject) {
>          connect(m_backendObject, SIGNAL(destroyed(QObject *)),
>                  this, SLOT(_k_destroyed(QObject *)));
>      }
>
>      if (!m_ifaces.isEmpty()) {
>          foreach (DeviceInterface *iface, m_ifaces) {
>              delete iface->d_ptr->backendObject();
>              delete iface;
>          }
>
>          m_ifaces.clear();
>
>          if (!ref.deref()) deleteLater();
>      }
> }
>
> And why is it decreased once we reset the backend object?

It's a mistake, should obviously be decreased for each instance in m_ifaces,
not simply once (as we increased for each call to setInterface()). Committed
the change in trunk just now.

> 5)
>
> void Solid::DevicePrivate::setBackendObject(Ifaces::Device *object)
> {
> ...
>        foreach (DeviceInterface *iface, m_ifaces) {
>              delete iface->d_ptr->backendObject();
>              delete iface;
>          }
>
> Why do we delete iface->d_ptr->backendObject() here ...
>
> Solid::DevicePrivate::~DevicePrivate()
> {
>      qDeleteAll(m_ifaces);
> }
>
> ... and not here?

Obviously missing. Committed the change in trunk just now.

Regards.
--
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud patron of KDE, http://www.kdab.com