Cancel the EffectLoader query on clear

The Scripted and PluginEffectLoader perform locating all effects which
are to be loaded in a thread. When the EffectLoader gets cleared so far
the query did not get canceled. This resulted in effects maybe getting
loaded.

This problems shows on build.kde.org if the test is too fast and tears
down the Effect system while effects are still being queried.

Reviewed-By: David Edmundson

autotests/test_plugin_effectloader.cpp

@@ -58,6 +58,7 @@ private Q_SLOTS:
     void testLoadPluginEffect_data();
     void testLoadPluginEffect();
     void testLoadAllEffects();
+    void testCancelLoadAllEffects();
 };
 
 void TestPluginEffectLoader::testHasEffect_data()
@@ -381,5 +382,32 @@ void TestPluginEffectLoader::testLoadAllEffects()
     spy.clear();
 }
 
+void TestPluginEffectLoader::testCancelLoadAllEffects()
+{
+    // this test verifies that no test gets loaded when the loader gets cleared
+    MockEffectsHandler mockHandler(KWin::OpenGL2Compositing);
+    KWin::PluginEffectLoader loader;
+    loader.setPluginSubDirectory(QString());
+
+    // prepare the configuration to hard enable/disable the effects we want to load
+    KSharedConfig::Ptr config = KSharedConfig::openConfig(QString(), KConfig::SimpleConfig);
+    KConfigGroup plugins = config->group("Plugins");
+    plugins.writeEntry(QStringLiteral("fakeeffectpluginEnabled"), true);
+    plugins.sync();
+
+    loader.setConfig(config);
+
+    qRegisterMetaType<KWin::Effect*>();
+    QSignalSpy spy(&loader, &KWin::PluginEffectLoader::effectLoaded);
+    QVERIFY(spy.isValid());
+
+    loader.queryAndLoadAll();
+    loader.clear();
+
+    // Should not load any effect
+    QVERIFY(!spy.wait(100));
+    QVERIFY(spy.isEmpty());
+}
+
 QTEST_MAIN(TestPluginEffectLoader)
 #include "test_plugin_effectloader.moc"

autotests/test_scripted_effectloader.cpp

@@ -72,6 +72,7 @@ private Q_SLOTS:
     void testLoadScriptedEffect_data();
     void testLoadScriptedEffect();
     void testLoadAllEffects();
+    void testCancelLoadAllEffects();
 };
 
 void TestScriptedEffectLoader::testHasEffect_data()
@@ -387,5 +388,32 @@ void TestScriptedEffectLoader::testLoadAllEffects()
     QCOMPARE(loadedEffects.at(1), kwin4 + QStringLiteral("scalein"));
 }
 
+void TestScriptedEffectLoader::testCancelLoadAllEffects()
+{
+    // this test verifies that no test gets loaded when the loader gets cleared
+    MockEffectsHandler mockHandler(KWin::XRenderCompositing);
+    KWin::ScriptedEffectLoader loader;
+
+    // prepare the configuration to hard enable/disable the effects we want to load
+    KSharedConfig::Ptr config = KSharedConfig::openConfig(QString(), KConfig::SimpleConfig);
+    const QString kwin4 = QStringLiteral("kwin4_effect_");
+    KConfigGroup plugins = config->group("Plugins");
+    plugins.writeEntry(kwin4 + QStringLiteral("scaleinEnabled"), true);
+    plugins.sync();
+
+    loader.setConfig(config);
+
+    qRegisterMetaType<KWin::Effect*>();
+    QSignalSpy spy(&loader, &KWin::ScriptedEffectLoader::effectLoaded);
+    QVERIFY(spy.isValid());
+
+    loader.queryAndLoadAll();
+    loader.clear();
+
+    // Should not load any effect
+    QVERIFY(!spy.wait(100));
+    QVERIFY(spy.isEmpty());
+}
+
 QTEST_MAIN(TestScriptedEffectLoader)
 #include "test_scripted_effectloader.moc"

effectloader.cpp

@@ -258,9 +258,12 @@ bool ScriptedEffectLoader::loadEffect(const KPluginMetaData &effect, LoadEffectF
 
 void ScriptedEffectLoader::queryAndLoadAll()
 {
+    if (m_queryConnection) {
+        return;
+    }
     // perform querying for the services in a thread
     QFutureWatcher<QList<KPluginMetaData>> *watcher = new QFutureWatcher<QList<KPluginMetaData>>(this);
-    connect(watcher, &QFutureWatcher<QList<KPluginMetaData>>::finished, this,
+    m_queryConnection = connect(watcher, &QFutureWatcher<QList<KPluginMetaData>>::finished, this,
         [this, watcher]() {
             const auto effects = watcher->result();
             for (auto effect : effects) {
@@ -270,6 +273,7 @@ void ScriptedEffectLoader::queryAndLoadAll()
                 }
             }
             watcher->deleteLater();
+            m_queryConnection = QMetaObject::Connection();
         },
         Qt::QueuedConnection);
     watcher->setFuture(QtConcurrent::run(this, &ScriptedEffectLoader::findAllEffects));
@@ -296,7 +300,8 @@ KPluginMetaData ScriptedEffectLoader::findEffect(const QString &name) const
 
 void ScriptedEffectLoader::clear()
 {
-    // TODO: cancel future
+    disconnect(m_queryConnection);
+    m_queryConnection = QMetaObject::Connection();
     m_queue->clear();
 }
 
@@ -432,9 +437,12 @@ bool PluginEffectLoader::loadEffect(const KPluginMetaData &info, LoadEffectFlags
 
 void PluginEffectLoader::queryAndLoadAll()
 {
+    if (m_queryConnection) {
+        return;
+    }
     // perform querying for the services in a thread
     QFutureWatcher<QVector<KPluginMetaData>> *watcher = new QFutureWatcher<QVector<KPluginMetaData>>(this);
-    connect(watcher, &QFutureWatcher<QVector<KPluginMetaData>>::finished, this,
+    m_queryConnection = connect(watcher, &QFutureWatcher<QVector<KPluginMetaData>>::finished, this,
         [this, watcher]() {
             const auto effects = watcher->result();
             for (const auto &effect : effects) {
@@ -444,6 +452,7 @@ void PluginEffectLoader::queryAndLoadAll()
                 }
             }
             watcher->deleteLater();
+            m_queryConnection = QMetaObject::Connection();
         },
         Qt::QueuedConnection);
     watcher->setFuture(QtConcurrent::run(this, &PluginEffectLoader::findAllEffects));
@@ -461,7 +470,8 @@ void PluginEffectLoader::setPluginSubDirectory(const QString &directory)
 
 void PluginEffectLoader::clear()
 {
-    // TODO: cancel future
+    disconnect(m_queryConnection);
+    m_queryConnection = QMetaObject::Connection();
     m_queue->clear();
 }
 

effectloader.h

@@ -323,6 +323,7 @@ private:
     KPluginMetaData findEffect(const QString &name) const;
     QStringList m_loadedEffects;
     EffectLoadQueue< ScriptedEffectLoader, KPluginMetaData > *m_queue;
+    QMetaObject::Connection m_queryConnection;
 };
 
 class PluginEffectLoader : public AbstractEffectLoader
@@ -350,6 +351,7 @@ private:
     QStringList m_loadedEffects;
     EffectLoadQueue< PluginEffectLoader, KPluginMetaData> *m_queue;
     QString m_pluginSubDirectory;
+    QMetaObject::Connection m_queryConnection;
 };
 
 class EffectLoader : public AbstractEffectLoader