Proxy models are really really great (They crash!)

Developments are ongoing in the quest for better testing methods for proxy models. I wrote before about testing a proxy model by visual inspection in the proxymodeltestapp. For identifying the sources of issues this has generally worked really well, but the involvement of human interaction means that it doesn’t scale and is error prone. Unit testing fares better in that regard because they can be run automatically without a human operator or observer, are faster than unit testing and are a persistent test for regressions so they allow for broad refactoring if needed for performance reasons.

This is not a very screenshot-able subject, so I have added some cat pictures at the suggestion of and for the amusement of tokoe.

Getting the blues

The biggest problem with Qt Model View is the amount of API that must be implemented to write a proxy model. Aside from implementing the QAbstractItemModel interface and the small additional interface in QAbstracyProxyModel, an observer interface for the source model must also be implemented. Source models emit signals such as rowsAboutToBeInserted and rowsInserted, and similar signals for removing, moving, restructuring the entire layout and a complete reset of the source model. Depending on the proxy model implementation, it may also be relevant that items with children are inserted, or that an entire subtree is removed or moved. Proxy models might also have boundary errors if items are removed or inserted at the top or bottom of an existing list, or if a row is moved from a visible row to an invisible row or vice-versa.

There are a large but finite number of updates that a source model can signal to a proxy model which must be reacted to in some way (even if the reaction of the proxy model is “do nothing”, if for example the inserted row is filtered out). It should be possible to write unit tests for proxy models such that the signals emitted by the source model are always the same, predictable and comprehensive.

My first attempt at doing so resulted in a unit test suite which commanded a source model through a sequence of steps covering insertion, moving, removal and layout changes. This did work in that it was possible to find bugs with it, but it did not scale well to different configurations of the proxy model under test and was inflexible to adding new tests. I started writing a different test file per configuration (kselectionproxymodeltest-onlyselectedchildren and kselectionproxymodeltest-selectedbranches for example, and they didn’t cover different selections), and adding tests anywhere but the end of the sequence meant updating all testcases in all test classes. Additionally, the sequence of tests did not make bug finding any easier. Executing a single test for one data point in the test was possible in most, but not all cases, and even then, all previous tests in the sequence would be executed before the particular data test of interest, causing a lot of noise.

New technology

My latest venture is to create tests which always initialize the source model to a known state and execute a single data driven test. Each data test is then standalone and can be executed independently or as part of a specified list. The minimal nature of the tests means that the developer can write a failing test and then easily execute only the failing code without first having to skip through a sequence of steps of prior tests.

The suite for executing, running and verifying the tests has also had many improvements. There are now predefined test data for indicating things like “This signal should be forwarded as-is through the proxy model”, which would be the case for a QSortFilterProxyModel without sorting or filtering enabled. This allows creation of very concise tests.

Untold opportunity

The configuration of a proxy model adds an extra dimension to how it is expected to behave in response to a stimulus from the source model. A QSortFilterProxyModel can have various filtering and sorting rules, although many could be described as generalizations of other tests. The behaviour of a KSelectionProxyModel is determined by the selection in a QItemSelectionModel and by the behaviour configured on the proxy model.

The way these large variety of configuration possibilities is handled is to define test data in template specializations of the test methods. For example in testing the KSelectionProxyModel there could be template specialisations like:

template<>
void TestData<SelectionStrategy<9>, KSelectionProxyModel::ExactSelection>::testRemoveFromTopLevelData()
{
// test data for when 9 is selected and the configuration of the proxy is ExactSelection
}

template<>
void TestData<SelectionStrategy<4, 7>, KSelectionProxyModel::SubTrees>::testRemoveFromTopLevelData()
{
// test data for when 4 and 7 are selected and the configuration of the proxy is SubTrees
}

In perhaps more familiar terms, QSortFilterProxyModels for different filter regexps could be specified like this:

// Need external linkage
extern const QRegExp re1_2_4(“1|2|4”);
extern const QRegExp re7_12_24(“7|12|24”);

template<>
void TestData<re1_2_4>::testRemoveFromTopLevelData()
{
// test data for when the filter regexp is “1|2|4”
}

template<>
void TestData<re7_12_24>::testRemoveFromTopLevelData()
{
// test data for when the filter regexp is “7|12|24”
}

In each case, the test data is a description of the signals expected to be emitted by the proxy and the expected update patterns to QPersistentModelIndexes in the proxy.

A consequence of using template specialization is that it allows partial implementation of test code for new proxy model configurations. For example, if a new bug is discovered when inserting rows for a particular implementation, but removing and moving rows are not affected, a test specialization for the relevant filter could be implemented for the insert test method only, leaving the others as the default implementation.

Proxy models without crashes

One of the more difficult crashes to track down in models or proxies looks something like this:

ASSERT failure in QPersistentModelIndex::~QPersistentModelIndex: "persistent model indexes corrupted",
file kernel/qabstractitemmodel.cpp, line 544

which can happen on shutdown, or with an innocent mouse movement some time after changing the model in some way by inserting or moving rows for example.

QModelIndexes updated over event loop returns

This happens when internal QPersistentModelIndexes are not updated consistently with what is reported to the outside via signals, and an observer creates a new QPersistentModelIndex as a result, or the proxy updates an existing persistent index to a conflicting position. In the test suite, this is covered by having different instances of the tests creating persistent indexes at different times. Each test is run twice- once for each persistent index taking strategy, that is creating QPersistentModelIndexes before running the test, and creating them during the test when the model emits rowsAboutToBeInserted for example. Additionally, the test suite monitors all QPersistentModelIndexes before and after running each test to ensure that updates are consistent with emitted signals.

Look up the index and double check

Another aspect of correct generic proxy models is that they should work well with other proxy models. To ensure this the test suite is run in a further two configurations for when the source model of the proxy under test is a “base” QAbstractItemModel or an intermediate QSortFilterProxyModel. That means that each data test in the suite is executed 4 times! The tests of course verify that the model() of all indexes always indicates the correct model.

Vigourous testing is concise

Currently the KSelectionProxyModel test executes tests for 1122 different data points, but that will get much larger soon when more configurations are tested.

The QtTest module provides a test suite for running a test with a particular test data set already. It is possible to run the test executable on the command line with arguments for the exact test data point required. The proxy model test suite extends that system by making it possible to run all available tests by running the ./kselectionproxymodeltest executable. When finding bugs it is useful to write new tests and execute only the failing code to discover the fix. The proxy model test suite allows executing a particular test for all configurations of the proxy, or all tests for just one configuration of the proxy, or some combination of tests for a selection of configurations.

In short, the new test suite is very versatile.

Advertisement

The Mother Goose Plugin Loader Pattern

Over the last few weeks I’ve been working on getting Grantlee finally into a releasable state for a 0.1 release.

Part of that has been to change the Granltee::Engine to not be a singleton anymore. Engine was a singleton because it managed plugin libraries which may be accessed my multiple Template objects at the same time. When the Engine was deleted it in turn deleted all plugins it had loaded. That worked fine when Engine was a singleton, but after this change I was getting multiple deletions when using multiple Engines.

It turns out that QPluginLoader maintains its own cache of plugins and only loads them once. Great! The only problem is deleting the plugin. My first thought was to put the plugin in a QSharedPointer. As that is reference counted, it would delete the plugin when nothing refers to it anymore.

  QPluginLoader loader("/path/to/plugin");
  QSharedPointer myPlugin = QSharedPointer( loader.instance() );

The problem is that two Engine objects create two shared pointers. The QPluginLoader caching meant that I was creating two shared pointers for one raw pointer. That leads to a problem similar to this:

  int* num_p;
  QSharedPointer p1(nump);
  QSharedPointer p2(nump);

  // ref counts of p1 and p2 are both 1.

  p1.clear(); // ref count drops to 0. p1 deletes num_p
  p2.clear(); // ref count drops to 0. p2 tries to delete num_p but it's a dangling pointer.

Hmm, how to resolve that so that I delete the plugins, and I get reference counting so that they are not deleted multiple times?

QPluginLoader has its own ref counting and will delete the plugin when unload() has been called for it for each QPluginLoader instance that has a handle on it. unload() is not called automatically in the QPluginLoader destructor so a plugin is never automatically deleted.

So the trick is to treat the QPluginLoader as a smart pointer. Each time you want a plugin, create a new QPluginLoader for it, store that, and when you no longer need the plugin, call unload() on each QPluginLoader. When the reference count drops to 0, the QPluginLoader will delete the plugin.

MyClass::~MyClass()
{
  foreach(QPluginLoader *loader, m_loaders)
  {
    loader->unload();
  }
  qDeleteAll(m_loaders);
}

/**
  The caller does not have to delete the returned plugin.
*/
MyPluginInterface* MyClass::loadPlugin( const QString &name )
{
  QPluginLoader *loader = new QPluginLoader( name, this );
  MyPluginInterface *plugin = qobject_cast( loader->instance() );
  m_loaders.append(loader);
  return plugin;
}

void someMethod()
{
  MyClass object1;

  object1.loadPlugin("plugin1"); // plugin1 ref count == 1
  object1.loadPlugin("plugin2"); // plugin2 ref count == 1
  if (true)
  {
    MyClass object2;
    object2.loadPlugin("plugin1"); // plugin1 ref count == 2
    object2.loadPlugin("plugin3"); // plugin1 ref count == 1
  } // object2 destructor calls unload() for each loader. 
    // plugin2 ref count drops to 1. plugin3 ref count drops to 0. plugin3 deleted.

} // object1 destructor calls unload() for each loader. plugin1 and 
  // plugin2 ref counts drop to 0. Both are deleted.

This means that there should be no attempt to use the plugins returned by loadPlugin after the MyClass has been deleted:

  MyPluginInterface *plugin;
  if (true)
  {
    MyClass object1;
    plugin = object1.loadPlugin("plugin1");
  } // object1 destructor deletes plugin.

  plugin->someMethod(); // It's already deleted. Crash.

In the case of Grantlee, the Engine manages the plugins and deletes them in its destructor. Applications never handle plugins directly, but the Engine is responsible for all of them.

As a final and reusable convenience, I wrote a Grantlee::PluginPointer class to load and manage plugins. It wraps a QSharedPointer and has a custom deleter which calls unload() when needed. It can be used like a normal plugin, but it will always be valid and will be deleted when the application exits at the latest. That means I don’t have to worry about the plugin being unloaded and deleted before I want to use it.

Reading all that back I can’t help thinking of the house that Jack built.

// This is the plugin that you want.
TagLibraryInterface *myPlugin;

// This is the QPluginLoader to load the plugin that you want.
QPluginLoader;
                  
// This is the shared pointer to manage the QPluginLoader to 
// load the plugin that you want.
QSharedPointer;

// This is the convenience pointer wrapping the shared pointer to manage the QPluginLoader to 
// load the plugin that you want
Grantlee::PluginPointer;       

// This is the interface to provide the convenience pointer
// wrapping the shared pointer to manage the QPluginLoader to
// load the plugin that you want.
Grantlee::Engine;               

So it’s still a bit convoluted. I don’t see reports from anyone else to hit this problem, and the

The QPluginLoader docs are not very clear on where the responsibility is to delete a loaded plugin, but running this test in valgrind is enough for me. I may have missed something though?

At any rate, I call this the Mother Goose Plugin Loader Pattern.