Solid DevicePrivate object ownership
jpalecek at web.de
Fri Mar 12 16:56:51 GMT 2010
On Mon, 22 Feb 2010 15:31:23 +0100, Kevin Ottens <ervin at kde.org> wrote:
> 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
>> deleted in this case (unless the device is removed).
> Yes, that's the intent. Once you did something advanced with a device
> 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
>> 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
>> 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
>> 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
> was handled and never got to actually finish the job... Volunteer spare
> can sometime drop quickly which has been the case for me regarding
> (which is mainly a one man show for now).
I understand this - yet I'd prefer someone beyond you (the community?)
would take care of libsolid (doing things like code review etc.), as it's
code deployed on many computers with kdelibs.
>> 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
>> natural to handle the deletion of its backend object, because existence
>> the backendObject beyond the lifetime of its DevicePrivate is
>> (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.
Thanks for committing it. However, won't there be any commit in the 4.4
>> 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
>> scheme that will delete the backend object when it's no longer needed.
>> patch changes it by making a copy in createDevice and managing
>> FakeDevice::Private by a shared pointer. I think this is a reasonable
>> 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.
Using Opera's revolutionary e-mail client: http://www.opera.com/mail/
More information about the kde-core-devel