introduce KDecorationPlugins::canLoad()

so we don't mess up our plugins on reload
also invoke it by loadPlugin()
and fix some KLibrary memleaks

REVIEW: 105499
BUG: 303247

kcmkwin/kwindecoration/decorationmodel.cpp

@@ -94,9 +94,7 @@ void DecorationModel::findDecorations()
                             findAuroraeThemes();
                             continue;
                         }
-                        if (m_plugins->loadPlugin(libName))
+                        if (!m_plugins->canLoad(libName))
-                            m_plugins->destroyPreviousPlugin();
-                        else
                             continue;
                         DecorationModelData data;
                         data.name = desktopFile.readName();
@@ -197,7 +195,8 @@ QVariant DecorationModel::data(const QModelIndex& index, int role) const
         if (m_plugins->loadPlugin(m_decorations[ index.row()].libraryName) &&
                 m_plugins->factory() != NULL) {
             foreach (KDecorationDefines::BorderSize size, m_plugins->factory()->borderSizes())   // krazy:exclude=foreach
-            sizes << int(size) ;
+                sizes << int(size);
+            m_plugins->destroyPreviousPlugin();
         }
         return sizes;
     }
@@ -312,13 +311,15 @@ void DecorationModel::regeneratePreview(const QModelIndex& index, const QSize& s
 
         document.setHtml(html);
         bool enabled = false;
-        if (m_plugins->loadPlugin(data.libraryName) && m_preview->recreateDecoration(m_plugins)) {
+        bool loaded;
+        if ((loaded = m_plugins->loadPlugin(data.libraryName)) && m_preview->recreateDecoration(m_plugins)) {
             enabled = true;
             m_preview->enablePreview();
         } else {
             m_preview->disablePreview();
         }
-        m_plugins->destroyPreviousPlugin();
+        if (loaded)
+            m_plugins->destroyPreviousPlugin();
         if (enabled) {
             m_preview->resize(size);
             m_preview->setTempButtons(m_plugins, m_customButtons, m_leftButtons, m_rightButtons);

libkdecorations/kdecoration_plugins_p.cpp

@@ -98,71 +98,60 @@ KDecoration* KDecorationPlugins::createDecoration(KDecorationBridge* bridge)
     return NULL;
 }
 
-// returns true if plugin was loaded successfully
+// tests whether the plugin can be loaded
-bool KDecorationPlugins::loadPlugin(QString nameStr)
+bool KDecorationPlugins::canLoad(QString nameStr, KLibrary **loadedLib)
 {
-    KConfigGroup group(config, QString("Style"));
+    if (nameStr.isEmpty())
-    if (nameStr.isEmpty()) {
+        return false; // we can't load that
-        nameStr = group.readEntry("PluginLib", defaultPlugin);
+
+    // Check if this library is not already loaded.
+    if (pluginStr == nameStr) {
+        if (loadedLib) {
+            *loadedLib = library;
+        }
+        return true;
     }
+
+    KConfigGroup group(config, QString("Style"));
     if (group.readEntry<bool>("NoPlugin", false)) {
         error(i18n("Loading of window decoration plugin library disabled in configuration."));
         return false;
     }
 
-    KLibrary *oldLibrary = library;
-    KDecorationFactory* oldFactory = fact;
-
     KLibrary libToFind(nameStr);
     QString path = libToFind.fileName();
     kDebug(1212) << "kwin : path " << path << " for " << nameStr;
 
-    // If the plugin was not found, try to find the default
     if (path.isEmpty()) {
-        nameStr = defaultPlugin;
-        KLibrary libToFind(nameStr);
-        path = libToFind.fileName();
-    }
-
-    // If no library was found, exit kwin with an error message
-    if (path.isEmpty()) {
-        error(i18n("No window decoration plugin library was found."));
         return false;
     }
 
-    // Check if this library is not already loaded.
-    if (pluginStr == nameStr)
-        return true;
-
     // Try loading the requested plugin
-    library = new KLibrary(path);
+    KLibrary *lib = new KLibrary(path);
 
-    // If that fails, fall back to the default plugin
+    if (!lib)
-trydefaultlib:
-    if (!library) {
-        kDebug(1212) << " could not load library, try default plugin again";
-        nameStr = defaultPlugin;
-        if (pluginStr == nameStr)
-            return true;
-        KLibrary libToFind(nameStr);
-        path = libToFind.fileName();
-        if (!path.isEmpty())
-            library = new KLibrary(path);
-    }
-
-    if (!library) {
-        error(i18n("The default decoration plugin is corrupt "
-                   "and could not be loaded."));
         return false;
-    }
 
-    create_ptr = NULL;
+    // TODO this would be a nice shortcut, but for "some" reason QtCurve with wrong ABI slips through
-    version_ptr = 0;
+    // TODO figure where it's loaded w/o being unloaded and check whether that can be fixed.
+#if 0
+    if (lib->isLoaded()) {
+        if (loadedLib) {
+            *loadedLib = lib;
+        }
+        return true;
+    }
+#endif
+    // so we check whether this lib was loaded before and don't unload it in case
+    bool wasLoaded = lib->isLoaded();
+
+    KDecorationFactory*(*cptr)() = NULL;
+    int (*vptr)()  = NULL;
     int deco_version = 0;
-    KLibrary::void_function_ptr version_func = library->resolveFunction("decoration_version");
+    KLibrary::void_function_ptr version_func = lib->resolveFunction("decoration_version");
     if (version_func) {
-        version_ptr = (int(*)())version_func;
+        vptr = (int(*)())version_func;
-        deco_version = version_ptr();
+        deco_version = vptr();
     } else {
         // block some decos known to link the unstable API but (for now) let through other legacy stuff
         const bool isLegacyStableABI = !(nameStr.contains("qtcurve", Qt::CaseInsensitive) ||
@@ -178,30 +167,77 @@ trydefaultlib:
         kWarning(1212) << "****** Please use the KWIN_DECORATION macro in extern \"C\" to get this decoration loaded in future versions of kwin";
     }
     if (deco_version != KWIN_DECORATION_API_VERSION) {
-        if (nameStr != defaultPlugin) {
+        if (version_func)
-            if (version_func)
+            kWarning(1212) << i18n("The library %1 has wrong API version %2", path, deco_version);
-                kWarning(1212) << i18n("The library %1 has wrong API version %2", path, deco_version);
+        lib->unload();
-            library->unload();
+        delete lib;
-            library = NULL;
+        return false;
-            goto trydefaultlib;
+    }
+
+    KLibrary::void_function_ptr create_func = lib->resolveFunction("create_factory");
+    if (create_func)
+        cptr = (KDecorationFactory * (*)())create_func;
+
+    if (!cptr) {
+        kDebug(1212) << i18n("The library %1 is not a KWin plugin.", path);
+        lib->unload();
+        delete lib;
+        return false;
+    }
+
+    if (loadedLib) {
+        *loadedLib = lib;
+    } else {
+        if (!wasLoaded)
+            lib->unload();
+        delete lib;
+    }
+    return true;
+}
+
+// returns true if plugin was loaded successfully
+bool KDecorationPlugins::loadPlugin(QString nameStr)
+{
+    KConfigGroup group(config, QString("Style"));
+    if (nameStr.isEmpty()) {
+        nameStr = group.readEntry("PluginLib", defaultPlugin);
+    }
+
+    // Check if this library is not already loaded.
+    if (pluginStr == nameStr)
+        return true;
+
+    KLibrary *oldLibrary = library;
+    KDecorationFactory* oldFactory = fact;
+
+    if (!canLoad(nameStr, &library)) {
+        // If that fails, fall back to the default plugin
+        nameStr = defaultPlugin;
+        if (!canLoad(nameStr, &library)) {
+            // big time trouble!
+            // -> exit kwin with an error message
+            error(i18n("The default decoration plugin is corrupt and could not be loaded."));
+            return false;
         }
     }
 
+    // guarded by "canLoad"
     KLibrary::void_function_ptr create_func = library->resolveFunction("create_factory");
     if (create_func)
         create_ptr = (KDecorationFactory * (*)())create_func;
-
+    if (!create_ptr) {  // this means someone probably attempted to load "some" kwin plugin/lib as deco
-    if (!create_ptr) {
+                        // and thus cheated on the "isLoaded" shortcut -> try the default and yell a warning
         if (nameStr != defaultPlugin) {
-            kDebug(1212) << i18n("The library %1 is not a KWin plugin.", path);
+            kWarning(1212) << i18n("The library %1 was attempted to be load as decoration plugin but is NOT", nameStr);
-            library->unload();
+            return loadPlugin(defaultPlugin);
-            library = NULL;
+        } else {
-            goto trydefaultlib;
+            // big time trouble!
+            // -> exit kwin with an error message
+            error(i18n("The default decoration plugin is corrupt and could not be loaded."));
+            return false;
         }
-        error(i18n("The library %1 is not a KWin plugin.", path));
-        library->unload();
-        return false;
     }
+
     fact = create_ptr();
     fact->checkRequirements(this);   // let it check what is supported
 
@@ -231,6 +267,7 @@ void KDecorationPlugins::destroyPreviousPlugin()
         delete old_fact;
         old_fact = NULL;
         old_library->unload();
+        delete old_library;
         old_library = NULL;
     }
 }

libkdecorations/kdecoration_plugins_p.h

@@ -48,6 +48,10 @@ class KWIN_EXPORT KDecorationPlugins
 public:
     KDecorationPlugins(const KSharedConfigPtr &cfg);
     virtual ~KDecorationPlugins();
+    /** Whether the plugin with @p name can be loaded
+     * if @p loadedLib is passed, the library is NOT unloaded and freed
+     * what is now your resposibility (intended for and used by below loadPlugin mainly) */
+    bool canLoad(QString name, KLibrary ** loadedLib = 0);
     bool loadPlugin(QString name);
     void destroyPreviousPlugin();
     KDecorationFactory* factory();
@@ -59,7 +63,6 @@ protected:
     QString defaultPlugin; // FRAME normalne protected?
 private:
     KDecorationFactory*(*create_ptr)();
-    int (*version_ptr)();
     KLibrary *library;
     KDecorationFactory* fact;
     KLibrary *old_library;