Problems with the new KSharedPtr
David Faure
faure at kde.org
Wed Oct 26 15:37:11 BST 2005
I forgot the reasoning for switching to refcount-in-the-pointer instead of
refcount-in-the-object, but after fixing kdelibs for 3 days due to the new KSharedPtr,
and it's still far from being completely fixed, I have very strong reservations against it.
One argument was probably "no need to let the object inherit from KShared anymore".
Well, that's nice for objects you don't have control over, and maybe some people might
need something like that in addition, no problem.
But for the kdelibs objects that did inherit KShared in kde3, I don't see that as a problem.
One problem with the new KSharedPtr is that it's just too easy to get it wrong.
KSharedPtr -> * -> KSharedPtr, and you get a double deletion (we were talking
on irc about making the constructor explicit to at least spot such problems).
Same thing if you independently create two KSharedPtr instances for the
same raw pointer.
Call that kind of code sloppy if you want, but it used to be fine so all of KDE
code does it, and it was perfectly fine to do it. See also below.
A bigger problem is that there's no way anymore to implement KSharedConfig.
There is a "pool of KSharedConfig instances", you ask for one by name and
you get a KSharedConfig::Ptr instance.
If you store the KSharedConfig objects previously created into a QList<KSharedConfig::Ptr>
then there is always a ref being kept on those objects, and they don't get deleted
when not used anymore.
If you store the KSharedConfig objects into a QList<KSharedConfig *> as before,
then when someone asks for a KSharedConfig that we have in the list, a second
KSharedConfig::Ptr gets created on the same raw pointer -> double deletion!
We would need to test isUnique() and remove from the list when it's true,
but when can we do that? I don't see a solution.
I'm asking for reverting to the KShared mechanism, it did work fine.
The only thing we gained in the new KSharedPtr was the conversion from
KSharedPtr<Base> to KSharedPtr<Derived> without ugly casts, but let's just
port that over to the old KSharedPtr (either the same way, auto conversion,
or with an explicit cast method).
--
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
More information about the kde-core-devel
mailing list