`make -j5 kritaflake`
At the end of June I finished copy-on-write vector layers. From the very beginning, I have been
researching into
possibilities to make kritaflake
implicitly sharable. In that post I mentioned the way
Sean Parent uses for Photoshop, and adapted it for the derived d-pointers in Flake.
Derived d-pointers
TL;DR: We got rid of it.
As I mentioned in the task page, derived d-pointers originally
in Flake are a barrier to implicit sharing. One of the reasons is that we need to write more code (either
KisSharedDescendent
wrapper class, or repeated code for virtual clone functions). Also, derived
d-pointers do not actually encapsulate the data in the parent classes -- for example, the members in
KoShapePrivate
are all accessible by descendents of KoShape
, say, KoShapeContainer
. That is probably
not how encapsulating should work. So in the end we decided to get rid of derived d-pointers in Flake.
This leads to one problem, however, in the class KoShapeGroup
. KoShapeGroup
is a descendent of KoShapeContainer
,
which owns a KoShapeContainerModel
that can be subclassed to control the behaviour when a child is added to or
removed from the container. KoShapeGroup
uses ShapeGroupContainerModel
which performs additional operations
specific to KoShapeGroup
.
After I merged my branch into master, it was said that Flake tests failed under address sanitizer (ASan). I
took a look and discovered that there was use after free in the class KoShapeGroup
, namely the use of its
d-pointer. The use is called by the destructor of KoShapeContainer
, which calls
KoShapeContainerModel::deleteOwnedShapes()
, which removes individual shapes
from the container, which then calls KoShapeGroup::invalidateSizeCache()
. The original situation was:
- destructor of
KoShapeGroup
was called; - members defined in
KoShapeGroup
got deleted (nothing, because everything is in the derived d-pointer which is defined inKoShape
); - destructor of
KoShapeContainer
was called, which callsd->model->deleteOwnedShapes()
; - then that of
KoShape
, which deletes all the private members.
But after the derived d-pointers are converted to normal ones, the calling sequence upon destruction becomes:
- destructor of
KoShapeGroup
was called; - members defined in
KoShapeGroup
got deleted (its own d-pointer); - destructor of
KoShapeContainer
was called, which callsd->model->deleteOwnedShapes()
; d->model
is aShapeGroupContainerModel
, which will callKoShapeGroup::invalidateSizeCache()
;- that last function accesses the d-pointer of
KoShapeGroup
, USE AFTER FREE.
In order to solve this problem we have to manually call model()->deleteOwnedShapes()
in the destructor
of KoShapeGroup
, at which time the d-pointer is still accessible.
q-pointers
TL;DR: We also got rid of it.
q-pointers are a method used in Qt to hide private methods from the header files, in order to improve
binary compatibility. q-pointers are stored in *Private classes (d
s), indicating the object that owns
this private instance. But this is, of course, conflicting with the principle of "sharing" because
the situation now is that multiple objects can own the same data. The q-pointers in flake is rather confusing
under such circumstances, since the private data cannot know which object is the caller.
To avoid this confusion, there are multiple ways:
- to move all the functions regarding q-pointers to the public classes;
- to pass the q-pointer every time when calling those functions in private classes; or
- to add another layer of "shared data" in the d-pointer and keep the q-pointers in the unshared part.
implicit sharing
To enable implicit sharing for the KoShape
hierarchy, the only thing left to be done is to
change the QScopedPointer<Private> d;
in the header file to QSharedDataPointer<Private> d;
and make the private classes inherit QSharedData
. This step is rather easy and then just run the
tests to make sure it does not break anything. Horray!