Refactored plugin-loader with improved dependency resolution
authorTobias Brunner <tobias@strongswan.org>
Sat, 8 Jun 2013 13:46:33 +0000 (15:46 +0200)
committerTobias Brunner <tobias@strongswan.org>
Tue, 11 Jun 2013 09:18:19 +0000 (11:18 +0200)
With the new implementation the plugins don't have to be listed in any
special order, dependencies are properly resolved.  The order only
matters if two plugins provide the same feature.

src/libcharon/plugins/stroke/stroke_list.c
src/libstrongswan/plugins/plugin_loader.c
src/libstrongswan/plugins/plugin_loader.h

index a2e1c80..e76afec 100644 (file)
@@ -1364,6 +1364,7 @@ static void list_plugins(FILE *out)
                                free(str);
                        }
                }
+               list->destroy(list);
        }
        enumerator->destroy(enumerator);
 }
index 109f78a..0549b37 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2012 Tobias Brunner
+ * Copyright (C) 2010-2013 Tobias Brunner
  * Copyright (C) 2007 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -30,6 +30,8 @@
 #include <utils/integrity_checker.h>
 
 typedef struct private_plugin_loader_t private_plugin_loader_t;
+typedef struct registered_feature_t registered_feature_t;
+typedef struct provided_feature_t provided_feature_t;
 typedef struct plugin_entry_t plugin_entry_t;
 
 /**
@@ -48,9 +50,14 @@ struct private_plugin_loader_t {
        linked_list_t *plugins;
 
        /**
-        * Hashtable for loaded features, as plugin_feature_t
+        * Hashtable for registered features, as registered_feature_t
         */
-       hashtable_t *loaded_features;
+       hashtable_t *features;
+
+       /**
+        * Loaded features (stored in reverse order), as provided_feature_t
+        */
+       linked_list_t *loaded;
 
        /**
         * List of names of loaded plugins
@@ -59,6 +66,80 @@ struct private_plugin_loader_t {
 };
 
 /**
+ * Registered plugin feature
+ */
+struct registered_feature_t {
+
+       /**
+        * The registered feature
+        */
+       plugin_feature_t *feature;
+
+       /**
+        * List of plugins providing this feature, as provided_feature_t
+        */
+       linked_list_t *plugins;
+};
+
+/**
+ * Hash a registered feature
+ */
+static bool registered_feature_hash(registered_feature_t *this)
+{
+       return plugin_feature_hash(this->feature);
+}
+
+/**
+ * Compare two registered features
+ */
+static bool registered_feature_equals(registered_feature_t *a,
+                                                                         registered_feature_t *b)
+{
+       return plugin_feature_equals(a->feature, b->feature);
+}
+
+/**
+ * Feature as provided by a plugin
+ */
+struct provided_feature_t {
+
+       /**
+        * Plugin providing the feature
+        */
+       plugin_entry_t *entry;
+
+       /**
+        * FEATURE_REGISTER or FEATURE_CALLBACK entry
+        */
+       plugin_feature_t *reg;
+
+       /**
+        * The provided feature (followed by dependencies)
+        */
+       plugin_feature_t *feature;
+
+       /**
+        * Maximum number of dependencies (following feature)
+        */
+       int dependencies;
+
+       /**
+        * TRUE if currently loading this feature (to prevent loops)
+        */
+       bool loading;
+
+       /**
+        * TRUE if feature loaded
+        */
+       bool loaded;
+
+       /**
+        * TRUE if feature failed to load
+        */
+       bool failed;
+};
+
+/**
  * Entry for a plugin
  */
 struct plugin_entry_t {
@@ -79,14 +160,9 @@ struct plugin_entry_t {
        void *handle;
 
        /**
-        * List of loaded features
-        */
-       linked_list_t *loaded;
-
-       /**
-        * List features failed to load
+        * List of features, as provided_feature_t
         */
-       linked_list_t *failed;
+       linked_list_t *features;
 };
 
 /**
@@ -99,8 +175,7 @@ static void plugin_entry_destroy(plugin_entry_t *entry)
        {
                dlclose(entry->handle);
        }
-       entry->loaded->destroy(entry->loaded);
-       entry->failed->destroy(entry->failed);
+       entry->features->destroy(entry->features);
        free(entry);
 }
 
@@ -177,14 +252,6 @@ static plugin_t *static_features_create(const char *name,
 }
 
 /**
- * Compare function for hashtable of loaded features.
- */
-static bool plugin_feature_equals_ptr(plugin_feature_t *a, plugin_feature_t *b)
-{
-       return a == b;
-}
-
-/**
  * create a plugin
  * returns: NOT_FOUND, if the constructor was not found
  *          FAILED, if the plugin could not be constructed
@@ -228,8 +295,7 @@ static status_t create_plugin(private_plugin_loader_t *this, void *handle,
        INIT(*entry,
                .plugin = plugin,
                .critical = critical,
-               .loaded = linked_list_create(),
-               .failed = linked_list_create(),
+               .features = linked_list_create(),
        );
        DBG2(DBG_LIB, "plugin '%s': loaded successfully", name);
        return SUCCESS;
@@ -238,8 +304,8 @@ static status_t create_plugin(private_plugin_loader_t *this, void *handle,
 /**
  * load a single plugin
  */
-static bool load_plugin(private_plugin_loader_t *this, char *name, char *file,
-                                               bool critical)
+static plugin_entry_t *load_plugin(private_plugin_loader_t *this, char *name,
+                                                                  char *file, bool critical)
 {
        plugin_entry_t *entry;
        void *handle;
@@ -248,7 +314,7 @@ static bool load_plugin(private_plugin_loader_t *this, char *name, char *file,
        {
                case SUCCESS:
                        this->plugins->insert_last(this->plugins, entry);
-                       return TRUE;
+                       return entry;
                case NOT_FOUND:
                        if (file)
                        {       /* try to load the plugin from a file */
@@ -256,7 +322,7 @@ static bool load_plugin(private_plugin_loader_t *this, char *name, char *file,
                        }
                        /* fall-through */
                default:
-                       return FALSE;
+                       return NULL;
        }
        if (lib->integrity)
        {
@@ -264,23 +330,33 @@ static bool load_plugin(private_plugin_loader_t *this, char *name, char *file,
                {
                        DBG1(DBG_LIB, "plugin '%s': failed file integrity test of '%s'",
                                 name, file);
-                       return FALSE;
+                       return NULL;
                }
        }
        handle = dlopen(file, RTLD_LAZY);
        if (handle == NULL)
        {
                DBG1(DBG_LIB, "plugin '%s' failed to load: %s", name, dlerror());
-               return FALSE;
+               return NULL;
        }
        if (create_plugin(this, handle, name, TRUE, critical, &entry) != SUCCESS)
        {
                dlclose(handle);
-               return FALSE;
+               return NULL;
        }
        entry->handle = handle;
        this->plugins->insert_last(this->plugins, entry);
-       return TRUE;
+       return entry;
+}
+
+/**
+ * Convert enumerated provided_feature_t to plugin_feature_t
+ */
+static bool feature_filter(void *null, provided_feature_t **provided,
+                                                  plugin_feature_t **feature)
+{
+       *feature = (*provided)->feature;
+       return (*provided)->loaded;
 }
 
 /**
@@ -289,10 +365,16 @@ static bool load_plugin(private_plugin_loader_t *this, char *name, char *file,
 static bool plugin_filter(void *null, plugin_entry_t **entry, plugin_t **plugin,
                                                  void *in, linked_list_t **list)
 {
-       *plugin = (*entry)->plugin;
+       plugin_entry_t *this = *entry;
+
+       *plugin = this->plugin;
        if (list)
        {
-               *list = (*entry)->loaded;
+               enumerator_t *features;
+               features = enumerator_create_filter(
+                                                       this->features->create_enumerator(this->features),
+                                                       (void*)feature_filter, NULL, NULL);
+               *list = linked_list_create_from_enumerator(features);
        }
        return TRUE;
 }
@@ -336,7 +418,6 @@ static char* loaded_plugins_list(private_plugin_loader_t *this)
        return buf;
 }
 
-
 /**
  * Check if a plugin is already loaded
  */
@@ -360,59 +441,171 @@ static bool plugin_loaded(private_plugin_loader_t *this, char *name)
 }
 
 /**
- * Check if a feature of a plugin is already loaded
+ * Forward declaration
+ */
+static void load_provided(private_plugin_loader_t *this,
+                                                 provided_feature_t *provided,
+                                                 int level);
+
+/**
+ * Used to find a loaded feature
+ */
+static bool is_feature_loaded(provided_feature_t *item)
+{
+       return item->loaded;
+}
+
+/**
+ * Used to find a loadable feature
  */
-static bool feature_loaded(private_plugin_loader_t *this, plugin_entry_t *entry,
-                                                  plugin_feature_t *feature)
+static bool is_feature_loadable(provided_feature_t *item)
 {
-       return entry->loaded->find_first(entry->loaded, NULL,
-                                                                        (void**)&feature) == SUCCESS;
+       return !item->loading && !item->loaded && !item->failed;
 }
 
 /**
- * Check if loading a feature of a plugin failed
+ * Find a loaded and matching feature
  */
-static bool feature_failed(private_plugin_loader_t *this, plugin_entry_t *entry,
-                                                  plugin_feature_t *feature)
+static bool loaded_feature_matches(registered_feature_t *a,
+                                                                  registered_feature_t *b)
 {
-       return entry->failed->find_first(entry->failed, NULL,
-                                                                        (void**)&feature) == SUCCESS;
+       if (plugin_feature_matches(a->feature, b->feature))
+       {
+               return b->plugins->find_first(b->plugins, (void*)is_feature_loaded,
+                                                                         NULL) == SUCCESS;
+       }
+       return FALSE;
 }
 
 /**
- * Check if dependencies are satisfied
+ * Find a loadable module that equals the requested feature
  */
-static bool dependencies_satisfied(private_plugin_loader_t *this,
-                                                               plugin_entry_t *entry, bool soft, bool report,
-                                                               plugin_feature_t *features, int count)
+static bool loadable_feature_equals(registered_feature_t *a,
+                                                                       registered_feature_t *b)
 {
+       if (plugin_feature_equals(a->feature, b->feature))
+       {
+               return b->plugins->find_first(b->plugins, (void*)is_feature_loadable,
+                                                                         NULL) == SUCCESS;
+       }
+       return FALSE;
+}
+
+/**
+ * Find a loadable module that matches the requested feature
+ */
+static bool loadable_feature_matches(registered_feature_t *a,
+                                                                        registered_feature_t *b)
+{
+       if (plugin_feature_matches(a->feature, b->feature))
+       {
+               return b->plugins->find_first(b->plugins, (void*)is_feature_loadable,
+                                                                         NULL) == SUCCESS;
+       }
+       return FALSE;
+}
+
+/**
+ * Returns a compatible plugin feature for the given depencency
+ */
+static bool find_compatible_feature(private_plugin_loader_t *this,
+                                                                       plugin_feature_t *dependency)
+{
+       registered_feature_t *feature, lookup = {
+               .feature = dependency,
+       };
+
+       feature = this->features->get_match(this->features, &lookup,
+                                                                          (void*)loaded_feature_matches);
+       return feature != NULL;
+}
+
+/**
+ * Load a registered plugin feature
+ */
+static void load_registered(private_plugin_loader_t *this,
+                                                       registered_feature_t *registered,
+                                                       int level)
+{
+       enumerator_t *enumerator;
+       provided_feature_t *provided;
+
+       enumerator = registered->plugins->create_enumerator(registered->plugins);
+       while (enumerator->enumerate(enumerator, &provided))
+       {
+               load_provided(this, provided, level);
+       }
+       enumerator->destroy(enumerator);
+}
+
+/**
+ * Try to load dependencies of the given feature
+ */
+static bool load_dependencies(private_plugin_loader_t *this,
+                                                         provided_feature_t *provided,
+                                                         int level)
+{
+       registered_feature_t *registered, lookup;
+       int indent = level * 2;
        int i;
 
        /* first entry is provided feature, followed by dependencies */
-       for (i = 1; i < count; i++)
+       for (i = 1; i < provided->dependencies; i++)
        {
-               plugin_feature_t *found;
-
-               if (features[i].kind != FEATURE_DEPENDS &&
-                       features[i].kind != FEATURE_SDEPEND)
+               if (provided->feature[i].kind != FEATURE_DEPENDS &&
+                       provided->feature[i].kind != FEATURE_SDEPEND)
                {       /* end of dependencies */
                        break;
                }
-               found = this->loaded_features->get_match(this->loaded_features,
-                                       &features[i], (hashtable_equals_t)plugin_feature_matches);
-               if (!found && (features[i].kind != FEATURE_SDEPEND || soft))
-               {
-                       if (report)
+
+               /* we load the feature even if a compatible one is already loaded,
+                * otherwise e.g. a specific database implementation loaded before
+                * another might cause a plugin feature loaded in-between to fail */
+               lookup.feature = &provided->feature[i];
+               do
+               {       /* prefer an exactly matching feature, could be omitted but
+                        * results in a more predictable behavior */
+                       registered = this->features->get_match(this->features,
+                                                                                &lookup,
+                                                                                (void*)loadable_feature_equals);
+                       if (!registered)
+                       {       /* try fuzzy matching */
+                               registered = this->features->get_match(this->features,
+                                                                                &lookup,
+                                                                                (void*)loadable_feature_matches);
+                       }
+                       if (registered)
                        {
-                               char *provide, *depend, *name;
+                               load_registered(this, registered, level);
+                       }
+                       /* we could stop after finding one but for dependencies like
+                        * DB_ANY it might be needed to load all matching features */
+               }
+               while (registered);
 
-                               name = entry->plugin->get_name(entry->plugin);
-                               provide = plugin_feature_get_string(&features[0]);
-                               depend = plugin_feature_get_string(&features[i]);
-                               DBG2(DBG_LIB, "feature %s in '%s' plugin has unsatisfied "
-                                        "dependency: %s", provide, name, depend);
-                               free(provide);
-                               free(depend);
+               if (!find_compatible_feature(this, &provided->feature[i]))
+               {
+                       char *name, *provide, *depend;
+                       bool soft = provided->feature[i].kind == FEATURE_SDEPEND;
+
+                       name = provided->entry->plugin->get_name(provided->entry->plugin);
+                       provide = plugin_feature_get_string(&provided->feature[0]);
+                       depend = plugin_feature_get_string(&provided->feature[i]);
+                       if (soft)
+                       {
+                               DBG2(DBG_LIB, "%*sfeature %s in plugin '%s' has unmet soft "
+                                        "dependency: %s", indent, "", provide, name, depend);
+                       }
+                       else
+                       {
+                               DBG1(DBG_LIB, "feature %s in plugin '%s' has unmet dependency: "
+                                        "%s", provide, name, depend);
+                       }
+                       free(provide);
+                       free(depend);
+                       if (soft)
+                       {       /* it's ok if we can't resolve soft dependencies */
+                               continue;
                        }
                        return FALSE;
                }
@@ -421,128 +614,133 @@ static bool dependencies_satisfied(private_plugin_loader_t *this,
 }
 
 /**
- * Check if a given feature is still required as dependency
+ * Load registered plugin features
  */
-static bool dependency_required(private_plugin_loader_t *this,
-                                                               plugin_feature_t *dep)
+static void load_feature(private_plugin_loader_t *this,
+                                                provided_feature_t *provided,
+                                                int level)
 {
-       enumerator_t *enumerator;
-       plugin_feature_t *features;
-       plugin_entry_t *entry;
-       int count, i;
-
-       enumerator = this->plugins->create_enumerator(this->plugins);
-       while (enumerator->enumerate(enumerator, &entry))
+       if (load_dependencies(this, provided, level))
        {
-               if (!entry->plugin->get_features)
-               {       /* features not supported */
-                       continue;
+               if (plugin_feature_load(provided->entry->plugin, provided->feature,
+                                                               provided->reg))
+               {
+                       provided->loaded = TRUE;
+                       /* insert first so we can unload the features in reverse order */
+                       this->loaded->insert_first(this->loaded, provided);
                }
-               count = entry->plugin->get_features(entry->plugin, &features);
-               for (i = 0; i < count; i++)
+               else
                {
-                       if (&features[i] != dep &&
-                               feature_loaded(this, entry, &features[i]))
-                       {
-                               while (++i < count && (features[i].kind == FEATURE_DEPENDS ||
-                                                                          features[i].kind == FEATURE_SDEPEND))
-                               {
-                                       if (plugin_feature_matches(&features[i], dep))
-                                       {
-                                               enumerator->destroy(enumerator);
-                                               return TRUE;
-                                       }
-                               }
-                       }
+                       provided->failed = TRUE;
                }
        }
-       enumerator->destroy(enumerator);
-       return FALSE;
+       else
+       {       /* TODO: we could check the current level and set a different flag when
+                * being loaded as dependency. If there are loops there is a chance the
+                * feature can be loaded later when loading the feature directly. */
+               provided->failed = TRUE;
+       }
 }
 
 /**
- * Load plugin features in correct order
+ * Load a provided feature
  */
-static int load_features(private_plugin_loader_t *this, bool soft, bool report)
+static void load_provided(private_plugin_loader_t *this,
+                                                 provided_feature_t *provided,
+                                                 int level)
 {
-       enumerator_t *enumerator;
-       plugin_feature_t *feature, *reg;
-       plugin_entry_t *entry;
-       int count, i, loaded = 0;
+       char *name, *provide;
+       int indent = level * 2;
 
+       if (provided->loaded || provided->failed)
+       {
+               return;
+       }
+       name = provided->entry->plugin->get_name(provided->entry->plugin);
+       provide = plugin_feature_get_string(provided->feature);
+       if (provided->loading)
+       {       /* prevent loop */
+               DBG2(DBG_LIB, "%*sloop detected while loading %s in plugin '%s'",
+                        indent, "", provide, name);
+               free(provide);
+               return;
+       }
+       DBG2(DBG_LIB, "%*sloading feature %s in plugin '%s'",
+                indent, "", provide, name);
+       free(provide);
+
+       provided->loading = TRUE;
+       load_feature(this, provided, level + 1);
+       provided->loading = FALSE;
+}
+
+/**
+ * Load registered plugin features
+ */
+static void load_features(private_plugin_loader_t *this)
+{
+       enumerator_t *enumerator, *inner;
+       plugin_entry_t *plugin;
+       provided_feature_t *provided;
+
+       /* we do this in plugin order to allow implicit dependencies to be resolved
+        * by reordering plugins */
        enumerator = this->plugins->create_enumerator(this->plugins);
-       while (enumerator->enumerate(enumerator, &entry))
+       while (enumerator->enumerate(enumerator, &plugin))
        {
-               if (!entry->plugin->get_features)
-               {       /* feature interface not supported */
-                       continue;
-               }
-               reg = NULL;
-               count = entry->plugin->get_features(entry->plugin, &feature);
-               for (i = 0; i < count; i++)
+               inner = plugin->features->create_enumerator(plugin->features);
+               while (inner->enumerate(inner, &provided))
                {
-                       switch (feature->kind)
-                       {
-                               case FEATURE_PROVIDE:
-                                       if (!feature_loaded(this, entry, feature) &&
-                                               !feature_failed(this, entry, feature) &&
-                                               dependencies_satisfied(this, entry, soft, report,
-                                                                                          feature, count - i))
-                                       {
-                                               if (plugin_feature_load(entry->plugin, feature, reg))
-                                               {
-                                                       this->loaded_features->put(this->loaded_features,
-                                                                                                          feature, feature);
-                                                       entry->loaded->insert_last(entry->loaded, feature);
-                                                       loaded++;
-                                               }
-                                               else
-                                               {
-                                                       entry->failed->insert_last(entry->failed, feature);
-                                               }
-                                       }
-                                       break;
-                               case FEATURE_REGISTER:
-                               case FEATURE_CALLBACK:
-                                       reg = feature;
-                                       break;
-                               default:
-                                       break;
-                       }
-                       feature++;
-               }
-               if (loaded && !report)
-               {       /* got new feature, restart from beginning of list */
-                       break;
+                       load_provided(this, provided, 0);
                }
+               inner->destroy(inner);
        }
        enumerator->destroy(enumerator);
-       return loaded;
 }
 
 /**
- * Try to unload plugin features on which is not depended anymore
+ * Register plugin features provided by the given plugin
  */
-static int unload_features(private_plugin_loader_t *this, plugin_entry_t *entry)
+static void register_features(private_plugin_loader_t *this,
+                                                         plugin_entry_t *entry)
 {
-       plugin_feature_t *feature, *reg = NULL;
-       int count, i, unloaded = 0;
+       plugin_feature_t *feature, *reg;
+       registered_feature_t *registered, lookup;
+       provided_feature_t *provided;
+       int count, i;
 
+       if (!entry->plugin->get_features)
+       {       /* feature interface not supported */
+               DBG1(DBG_LIB, "plugin '%s' does not provide features, deprecated",
+                        entry->plugin->get_name(entry->plugin));
+               return;
+       }
+       reg = NULL;
        count = entry->plugin->get_features(entry->plugin, &feature);
        for (i = 0; i < count; i++)
        {
                switch (feature->kind)
                {
                        case FEATURE_PROVIDE:
-                               if (feature_loaded(this, entry, feature) &&
-                                       !dependency_required(this, feature) &&
-                                       plugin_feature_unload(entry->plugin, feature, reg))
+                               lookup.feature = feature;
+                               registered = this->features->get(this->features, &lookup);
+                               if (!registered)
                                {
-                                       this->loaded_features->remove(this->loaded_features,
-                                                                                                 feature);
-                                       entry->loaded->remove(entry->loaded, feature, NULL);
-                                       unloaded++;
+                                       INIT(registered,
+                                               .feature = feature,
+                                               .plugins = linked_list_create(),
+                                       );
+                                       this->features->put(this->features, registered, registered);
                                }
+                               INIT(provided,
+                                       .entry = entry,
+                                       .feature = feature,
+                                       .reg = reg,
+                                       .dependencies = count - i,
+                               );
+                               registered->plugins->insert_last(registered->plugins,
+                                                                                                provided);
+                               entry->features->insert_last(entry->features, provided);
                                break;
                        case FEATURE_REGISTER:
                        case FEATURE_CALLBACK:
@@ -553,7 +751,54 @@ static int unload_features(private_plugin_loader_t *this, plugin_entry_t *entry)
                }
                feature++;
        }
-       return unloaded;
+}
+
+/**
+ * Unregister a plugin feature
+ */
+static void unregister_feature(private_plugin_loader_t *this,
+                                                          provided_feature_t *provided)
+{
+       registered_feature_t *registered, lookup;
+
+       lookup.feature = provided->feature;
+       registered = this->features->get(this->features, &lookup);
+       if (registered)
+       {
+               registered->plugins->remove(registered->plugins, provided, NULL);
+               if (registered->plugins->get_count(registered->plugins) == 0)
+               {
+                       this->features->remove(this->features, &lookup);
+                       registered->plugins->destroy(registered->plugins);
+                       free(registered);
+               }
+               else if (registered->feature == provided->feature)
+               {       /* update feature in case the providing plugin gets unloaded */
+                       provided_feature_t *first;
+
+                       registered->plugins->get_first(registered->plugins, (void**)&first);
+                       registered->feature = first->feature;
+               }
+       }
+       free(provided);
+}
+
+/**
+ * Unregister plugin features
+ */
+static void unregister_features(private_plugin_loader_t *this,
+                                                               plugin_entry_t *entry)
+{
+       provided_feature_t *provided;
+       enumerator_t *enumerator;
+
+       enumerator = entry->features->create_enumerator(entry->features);
+       while (enumerator->enumerate(enumerator, &provided))
+       {
+               entry->features->remove_at(entry->features, enumerator);
+               unregister_feature(this, provided);
+       }
+       enumerator->destroy(enumerator);
 }
 
 /**
@@ -561,44 +806,43 @@ static int unload_features(private_plugin_loader_t *this, plugin_entry_t *entry)
  */
 static bool missing_critical_features(private_plugin_loader_t *this)
 {
-       enumerator_t *enumerator;
+       enumerator_t *enumerator, *features;
        plugin_entry_t *entry;
        bool critical_failed = FALSE;
 
        enumerator = this->plugins->create_enumerator(this->plugins);
        while (enumerator->enumerate(enumerator, &entry))
        {
-               if (!entry->plugin->get_features)
-               {       /* feature interface not supported */
+               provided_feature_t *provided;
+               char *name, *provide;
+               int failed = 0;
+
+               if (!entry->plugin->get_features ||
+                       !entry->critical)
+               {       /* feature interface not supported, or not critical */
                        continue;
                }
-               if (entry->critical)
-               {
-                       plugin_feature_t *feature;
-                       char *name, *provide;
-                       int count, i, failed = 0;
 
-                       name = entry->plugin->get_name(entry->plugin);
-                       count = entry->plugin->get_features(entry->plugin, &feature);
-                       for (i = 0; i < count; i++, feature++)
-                       {
-                               if (feature->kind == FEATURE_PROVIDE &&
-                                       !feature_loaded(this, entry, feature))
-                               {
-                                       provide = plugin_feature_get_string(feature);
-                                       DBG2(DBG_LIB, "  failed to load %s in critical plugin '%s'",
-                                                provide, name);
-                                       free(provide);
-                                       failed++;
-                               }
-                       }
-                       if (failed)
+               name = entry->plugin->get_name(entry->plugin);
+               features = entry->features->create_enumerator(entry->features);
+               while (features->enumerate(features, &provided))
+               {
+                       if (!provided->loaded)
                        {
-                               DBG1(DBG_LIB, "failed to load %d feature%s in critical plugin "
-                                        "'%s'", failed, failed > 1 ? "s" : "", name);
-                               critical_failed = TRUE;
+                               provide = plugin_feature_get_string(provided->feature);
+                               DBG2(DBG_LIB, "  failed to load %s in critical plugin '%s'",
+                                        provide, name);
+                               free(provide);
+                               failed++;
                        }
                }
+               features->destroy(features);
+               if (failed)
+               {
+                       DBG1(DBG_LIB, "failed to load %d feature%s in critical plugin '%s'",
+                                failed, failed > 1 ? "s" : "", name);
+                       critical_failed = TRUE;
+               }
        }
        enumerator->destroy(enumerator);
 
@@ -606,7 +850,7 @@ static bool missing_critical_features(private_plugin_loader_t *this)
 }
 
 /**
- * Remove plugins that we were not able to load any features from.
+ * Remove plugins we were not able to load any plugin features from.
  */
 static void purge_plugins(private_plugin_loader_t *this)
 {
@@ -620,9 +864,13 @@ static void purge_plugins(private_plugin_loader_t *this)
                {       /* feature interface not supported */
                        continue;
                }
-               if (!entry->loaded->get_count(entry->loaded))
+               if (entry->features->find_first(entry->features,
+                                                                       (void*)is_feature_loaded, NULL) != SUCCESS)
                {
+                       DBG2(DBG_LIB, "unloading plugin '%s' without loaded features",
+                                entry->plugin->get_name(entry->plugin));
                        this->plugins->remove_at(this->plugins, enumerator);
+                       unregister_features(this, entry);
                        plugin_entry_destroy(entry);
                }
        }
@@ -641,10 +889,10 @@ METHOD(plugin_loader_t, add_static_features, void,
        INIT(entry,
                .plugin = plugin,
                .critical = critical,
-               .loaded = linked_list_create(),
-               .failed = linked_list_create(),
+               .features = linked_list_create(),
        );
        this->plugins->insert_last(this->plugins, entry);
+       register_features(this, entry);
 }
 
 METHOD(plugin_loader_t, load_plugins, bool,
@@ -664,6 +912,7 @@ METHOD(plugin_loader_t, load_plugins, bool,
        enumerator = enumerator_create_token(list, " ", " ");
        while (!critical_failed && enumerator->enumerate(enumerator, &token))
        {
+               plugin_entry_t *entry;
                bool critical = FALSE;
                char buf[PATH_MAX], *file = NULL;
                int len;
@@ -689,29 +938,22 @@ METHOD(plugin_loader_t, load_plugins, bool,
                        }
                        file = buf;
                }
-               if (!load_plugin(this, token, file, critical) && critical)
+               entry = load_plugin(this, token, file, critical);
+               if (entry)
+               {
+                       register_features(this, entry);
+               }
+               else if (critical)
                {
                        critical_failed = TRUE;
                        DBG1(DBG_LIB, "loading critical plugin '%s' failed", token);
                }
                free(token);
-               /* TODO: we currently load features after each plugin is loaded. This
-                * will not be necessary once we have features support in all plugins.
-                */
-               while (load_features(this, TRUE, FALSE))
-               {
-                       /* try load new features until we don't get new ones */
-               }
        }
        enumerator->destroy(enumerator);
        if (!critical_failed)
        {
-               while (load_features(this, FALSE, FALSE))
-               {
-                       /* enforce loading features, ignoring soft dependencies */
-               }
-               /* report missing dependencies */
-               load_features(this, FALSE, TRUE);
+               load_features(this);
                /* check for unloaded features provided by critical plugins */
                critical_failed = missing_critical_features(this);
                /* unload plugins that we were not able to load any features for */
@@ -725,44 +967,42 @@ METHOD(plugin_loader_t, load_plugins, bool,
        return !critical_failed;
 }
 
-METHOD(plugin_loader_t, unload, void,
-       private_plugin_loader_t *this)
+/**
+ * Unload plugin features, they are registered in reverse order
+ */
+static void unload_features(private_plugin_loader_t *this)
 {
        enumerator_t *enumerator;
+       provided_feature_t *provided;
        plugin_entry_t *entry;
-       linked_list_t *list;
 
-       /* unload plugins in reverse order, for those not supporting features */
-       list = linked_list_create();
-       while (this->plugins->remove_last(this->plugins, (void**)&entry) == SUCCESS)
+       enumerator = this->loaded->create_enumerator(this->loaded);
+       while (enumerator->enumerate(enumerator, &provided))
        {
-               list->insert_last(list, entry);
+               entry = provided->entry;
+               plugin_feature_unload(entry->plugin, provided->feature, provided->reg);
+               this->loaded->remove_at(this->loaded, enumerator);
+               entry->features->remove(entry->features, provided, NULL);
+               unregister_feature(this, provided);
        }
-       while (list->remove_last(list, (void**)&entry) == SUCCESS)
-       {
-               this->plugins->insert_first(this->plugins, entry);
-       }
-       list->destroy(list);
-       while (this->plugins->get_count(this->plugins))
+       enumerator->destroy(enumerator);
+}
+
+METHOD(plugin_loader_t, unload, void,
+       private_plugin_loader_t *this)
+{
+       plugin_entry_t *entry;
+
+       /* unload features followed by plugins, in reverse order */
+       unload_features(this);
+       while (this->plugins->remove_last(this->plugins, (void**)&entry) == SUCCESS)
        {
-               enumerator = this->plugins->create_enumerator(this->plugins);
-               while (enumerator->enumerate(enumerator, &entry))
-               {
-                       if (entry->plugin->get_features)
-                       {       /* supports features */
-                               while (unload_features(this, entry));
-                       }
-                       if (entry->loaded->get_count(entry->loaded) == 0)
-                       {
-                               if (lib->leak_detective)
-                               {       /* keep handle to report leaks properly */
-                                       entry->handle = NULL;
-                               }
-                               this->plugins->remove_at(this->plugins, enumerator);
-                               plugin_entry_destroy(entry);
-                       }
+               if (lib->leak_detective)
+               {       /* keep handle to report leaks properly */
+                       entry->handle = NULL;
                }
-               enumerator->destroy(enumerator);
+               unregister_features(this, entry);
+               plugin_entry_destroy(entry);
        }
        free(this->loaded_plugins);
        this->loaded_plugins = NULL;
@@ -824,7 +1064,8 @@ METHOD(plugin_loader_t, destroy, void,
        private_plugin_loader_t *this)
 {
        unload(this);
-       this->loaded_features->destroy(this->loaded_features);
+       this->features->destroy(this->features);
+       this->loaded->destroy(this->loaded);
        this->plugins->destroy(this->plugins);
        free(this->loaded_plugins);
        free(this);
@@ -848,11 +1089,11 @@ plugin_loader_t *plugin_loader_create()
                        .destroy = _destroy,
                },
                .plugins = linked_list_create(),
-               .loaded_features = hashtable_create(
-                                                               (hashtable_hash_t)plugin_feature_hash,
-                                                               (hashtable_equals_t)plugin_feature_equals_ptr, 64),
+               .loaded = linked_list_create(),
+               .features = hashtable_create(
+                                                       (hashtable_hash_t)registered_feature_hash,
+                                                       (hashtable_equals_t)registered_feature_equals, 64),
        );
 
        return &this->public;
 }
-
index 6a8f8f6..857bb2d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Tobias Brunner
+ * Copyright (C) 2012-2013 Tobias Brunner
  * Copyright (C) 2007 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
@@ -84,9 +84,9 @@ struct plugin_loader_t {
        /**
         * Create an enumerator over all loaded plugins.
         *
-        * In addition to the plugin, the enumerator returns a list of pointers to
-        * plugin features currently loaded (if the argument is not NULL).
-        * This list is to be read only.
+        * In addition to the plugin, the enumerator optionally provides a list of
+        * pointers to plugin features currently loaded.
+        * This list has to be destroyed.
         *
         * @return                              enumerator over plugin_t*, linked_list_t*
         */