`make -j5 kritaflake`

Posted on kdekritatechc++

Table of Contents 目录

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:

  1. destructor of KoShapeGroup was called;
  2. members defined in KoShapeGroup got deleted (nothing, because everything is in the derived d-pointer which is defined in KoShape);
  3. destructor of KoShapeContainer was called, which calls d->model->deleteOwnedShapes();
  4. 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:

  1. destructor of KoShapeGroup was called;
  2. members defined in KoShapeGroup got deleted (its own d-pointer);
  3. destructor of KoShapeContainer was called, which calls d->model->deleteOwnedShapes();
  4. d->model is a ShapeGroupContainerModel, which will call KoShapeGroup::invalidateSizeCache();
  5. 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 (ds), 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:

  1. to move all the functions regarding q-pointers to the public classes;
  2. to pass the q-pointer every time when calling those functions in private classes; or
  3. 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!