[Kde-hardware-devel] Solid DevicePrivate object ownership

[Kevin Ottens]

On Saturday 19 December 2009 01:39:21 Jiri Palecek wrote:
> On Tuesday 24 November 2009 21:24:39 Kevin Ottens wrote:
> > 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).
> 
> OK, that makes sense, however, that would mean the DevicePrivate is never
> deleted in this case (unless the device is removed).

Yes, that's the intent. Once you did something advanced with a device it's 
supposed to stay here until it's unplugged.
 
> The problem I found is that 1) and 2) are not true in actual code. The
> problem is that when a DevicePrivate ceases to exist, the DeviceManager
> won't have any reference on the (say) HalDevice, which will however never
> be deleted, and further requests for the same device will create a new
> HalDevice. For a proof of the behavior, apply attached patch "Proof of the
> leak", and run the changed test (halbasictest) under valgrind. On my
> system, it shows some 1.5 MB in "still reachable" memory, which is made by
> the leaked objects. Note that the objects are leaked regardless they are
> "still reachable", because the references leading to them are some
> callback references, that do not allow the objects to be ever used again
> (compare to the state without the patch).

Yup, mainly comes from the fact that I have been switching the way ownership 
was handled and never got to actually finish the job... Volunteer spare time 
can sometime drop quickly which has been the case for me regarding libsolid 
(which is mainly a one man show for now).

> I propose two ways of getting rid of this:
> 
> 1) Make the backendObject owned by the DevicePrivate
> 
> This solution strives to delete the backend objects as much as possible
> (eg. when the Device goes out of scope...). IMHO, for DevicePrivate it is
> natural to handle the deletion of its backend object, because existence of
> the backendObject beyond the lifetime of its DevicePrivate is nonsensical
> (except for FakeHw backend, more on that later), and there is a 1-1
> correspondence between DevicePrivate and backendObject. The DeviceManger
> can still manipulate the backendObject if the device is remove by doing
> setBackendObject. The patch that implements this approach is implemented
> by the "Fix solid DevicePrivate ... memory leak ... by moving its
> ownership... " (the largest one).

Definitely the preferred way as that complete what was left unfinished.

> The only problem with this is that the FakeHw Manager backend doesn't
> create an object in its createDevice function, but rather returns a
> pointer to a pre-existing object. This is of course incompatible with any
> scheme that will delete the backend object when it's no longer needed. The
> patch changes it by making a copy in createDevice and managing
> FakeDevice::Private by a shared pointer. I think this is a reasonable cost
> as I don't have any "fake" hardware in my computer, and it seems natural
> to me that something only needed for testing should adapt for changes
> needed for the production code to work.

Sure thing.

> Note that the patches are all based on 4.3.4.

Adapted the bits I was interested in and now committed.

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

KDAB - proud patron of KDE, http://www.kdab.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20100222/82ad1ac5/attachment-0001.sig