<p>Corey Farrell has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/7873">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">loader: Process module dependencies.<br><br>* Add string vectors for requires, optional_apis and enhances.<br>* Add reffed_deps module vector for holding references to dependencies.<br>* Initialize string vectors after final dlopen of each module.<br>* Free string vectors and clear references from reffed_deps in<br>  module_destroy.<br>* Create functions necessary to process module dependencies and enforce<br>  load order.<br><br>Module dependencies result in automatic references being managed by the<br>module loader.  This enforces unload order.<br><br>Change-Id: I9be08d1dd331aceadc1dcba00b804d71360b2fbb<br>---<br>M main/loader.c<br>1 file changed, 407 insertions(+), 23 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/73/7873/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/loader.c b/main/loader.c<br>index 88c1cda..8f15b07 100644<br>--- a/main/loader.c<br>+++ b/main/loader.c<br>@@ -89,6 +89,12 @@<br>        </managerEvent><br>  ***/<br> <br>+#if defined(OPTIONAL_API)<br>+#define HAS_OPTIONAL_API 1<br>+#else<br>+#define HAS_OPTIONAL_API 0<br>+#endif<br>+<br> #ifndef RTLD_NOW<br> #define RTLD_NOW 0<br> #endif<br>@@ -127,6 +133,21 @@<br>     int usecount;<br>         /*! List of users holding the module. */<br>      struct module_user_list users;<br>+<br>+    /*! List of required module names. */<br>+        struct ast_vector_string requires;<br>+   /*! List of optional api modules. */<br>+ struct ast_vector_string optional_apis;<br>+      /*! List of modules this enhances. */<br>+        struct ast_vector_string enhances;<br>+<br>+        /*!<br>+   * \brief Vector holding pointers to modules we have a reference to.<br>+  *<br>+    * When one module requires another, the required module gets added<br>+   * to this list with a reference.<br>+     */<br>+  struct module_vector reffed_deps;<br>     struct {<br>              /*! The module running and ready to accept requests. */<br>               unsigned int running:1;<br>@@ -161,6 +182,233 @@<br>         * >0 a_pri > b_pri<br>      */<br>   return a_pri - b_pri;<br>+}<br>+<br>+static struct ast_module *find_resource(const char *resource, int do_lock);<br>+<br>+/*!<br>+ * \internal<br>+ * \brief Add a reference from mod to dep.<br>+ *<br>+ * \param mod Owner of the new reference.<br>+ * \param dep Module to reference<br>+ * \param missing Vector to store name of \a dep if it is not running.<br>+ *<br>+ * This function returns failure if \a dep is not running and \a missing<br>+ * is NULL.  If \a missing is not NULL errors will only be returned for<br>+ * allocation failures.<br>+ *<br>+ * \retval 0 Success<br>+ * \retval -1 Failure<br>+ *<br>+ * \note Adding a second reference to the same dep will return success<br>+ *       without doing anything.<br>+ */<br>+static int module_reffed_deps_add(struct ast_module *mod, struct ast_module *dep,<br>+     struct ast_vector_const_string *missing)<br>+{<br>+ if (!dep->flags.running) {<br>+                return !missing ? -1 : AST_VECTOR_APPEND(missing, dep->info->name);<br>+    }<br>+<br>+ if (AST_VECTOR_GET_CMP(&mod->reffed_deps, dep, AST_VECTOR_ELEM_DEFAULT_CMP)) {<br>+                /* Skip duplicate. */<br>+                return 0;<br>+    }<br>+<br>+ if (AST_VECTOR_APPEND(&mod->reffed_deps, dep)) {<br>+              return -1;<br>+   }<br>+<br>+ ast_module_ref(dep);<br>+<br>+      return 0;<br>+}<br>+<br>+/*!<br>+ * \internal<br>+ * \brief Add references for modules that enhance a dependency.<br>+ *<br>+ * \param mod Owner of the new references.<br>+ * \param dep Module to check for enhancers.<br>+ * \param missing Vector to store name of any enhancer that is not running or declined.<br>+ *<br>+ * \retval 0 Success<br>+ * \retval -1 Failure<br>+ */<br>+static int module_reffed_deps_add_dep_enhancers(struct ast_module *mod,<br>+       struct ast_module *dep, struct ast_vector_const_string *missing)<br>+{<br>+ struct ast_module *cur;<br>+<br>+   AST_DLLIST_TRAVERSE(&module_list, cur, entry) {<br>+          if (cur->flags.declined) {<br>+                        continue;<br>+            }<br>+<br>+         if (!AST_VECTOR_GET_CMP(&cur->enhances, dep->info->name, !strcasecmp)) {<br>+                        /* dep is not enhanced by cur. */<br>+                    continue;<br>+            }<br>+<br>+         /* dep is enhanced by cur, therefore mod requires cur. */<br>+            if (module_reffed_deps_add(mod, cur, missing)) {<br>+                     return -1;<br>+           }<br>+    }<br>+<br>+ return 0;<br>+}<br>+<br>+/*!<br>+ * \internal<br>+ * \brief Add references to a list of dependencies.<br>+ *<br>+ * \param mod Owner of the new references.<br>+ * \param vec List of required modules to process<br>+ * \param missing Vector to store names of modules that are not running.<br>+ * \param ref_enhancers Reference all enhancers of each required module.<br>+ * \param isoptional Modules that are not loaded can be ignored.<br>+ *<br>+ * \retval 0 Success<br>+ * \retval -1 Failure<br>+ */<br>+static int module_deps_process_reqlist(struct ast_module *mod,<br>+        struct ast_vector_string *vec, struct ast_vector_const_string *missing,<br>+      int ref_enhancers, int isoptional)<br>+{<br>+       int idx;<br>+<br>+  for (idx = 0; idx < AST_VECTOR_SIZE(vec); idx++) {<br>+                const char *depname = AST_VECTOR_GET(vec, idx);<br>+              struct ast_module *dep = find_resource(depname, 0);<br>+<br>+               if (!dep || !dep->flags.running) {<br>+                        if (isoptional && !dep) {<br>+                            continue;<br>+                    }<br>+<br>+                 if (missing && !AST_VECTOR_APPEND(missing, depname)) {<br>+                               continue;<br>+                    }<br>+<br>+                 return -1;<br>+           }<br>+<br>+         if (module_reffed_deps_add(mod, dep, missing)) {<br>+                     return -1;<br>+           }<br>+<br>+         if (ref_enhancers && module_reffed_deps_add_dep_enhancers(mod, dep, missing)) {<br>+                      return -1;<br>+           }<br>+    }<br>+<br>+ return 0;<br>+}<br>+<br>+/*!<br>+ * \internal<br>+ * \brief Grab all references required to start the module.<br>+ *<br>+ * \param mod The module we're trying to start.<br>+ * \param missing Vector to store a list of missing dependencies.<br>+ *<br>+ * \retval 0 Success<br>+ * \retval -1 Failure<br>+ *<br>+ * \note module_list must be locked.<br>+ *<br>+ * \note Caller is responsible for initializing and freeing the vector on<br>+ *       success.  Elements are only safe read while module_list remains locked.<br>+ */<br>+static int module_deps_reference(struct ast_module *mod, struct ast_vector_const_string *missing)<br>+{<br>+ int res = 0;<br>+<br>+      /* Grab references to modules we enhance but not other enhancements. */<br>+      res |= module_deps_process_reqlist(mod, &mod->enhances, missing, 0, 0);<br>+<br>+    /* Grab references to modules we require plus enhancements. */<br>+       res |= module_deps_process_reqlist(mod, &mod->requires, missing, 1, 0);<br>+<br>+    /* Grab references to optional api modules including enhancements. */<br>+        res |= module_deps_process_reqlist(mod, &mod->optional_apis, missing, 1, HAS_OPTIONAL_API);<br>+<br>+        if (!res) {<br>+          return 0;<br>+    }<br>+<br>+ if (missing) {<br>+               /* We had an allocation error so the list was incomplete anyways. */<br>+         AST_VECTOR_FREE(missing);<br>+    }<br>+<br>+ return -1;<br>+}<br>+<br>+/*!<br>+ * \brief Recursively find required dependencies that are not running.<br>+ *<br>+ * \param mod Module to scan for dependencies.<br>+ * \param missingdeps Vector listing modules that must be started first.<br>+ *<br>+ * \retval 0 All dependencies resolved.<br>+ * \retval -1 Failed to resolve some dependencies.<br>+ *<br>+ * An error from this function usually means a required module is not even<br>+ * loaded.  This function is safe from infinite recursion, but dependency<br>+ * loops are not reported as an error from here.  On success missingdeps<br>+ * will contain a list of every module that needs to be running before this<br>+ * module can start.<br>+ */<br>+static int module_deps_missing_recursive(struct ast_module *mod, struct module_vector *missingdeps)<br>+{<br>+  int i = 0;<br>+   int res = -1;<br>+        struct ast_vector_const_string localdeps;<br>+    struct ast_module *tmp;<br>+<br>+   /*<br>+    * localdeps stores a copy of all dependencies that mod could not reference.<br>+  * First we discard modules that we've already found. We add all newly found<br>+      * modules to the missingdeps vector then scan them recursively.  This will<br>+   * ensure we quickly run out of stuff to do.<br>+  */<br>+  AST_VECTOR_INIT(&localdeps, 0);<br>+  if (module_deps_reference(mod, &localdeps)) {<br>+            goto clean_return;<br>+   }<br>+<br>+ while (i < AST_VECTOR_SIZE(&localdeps)) {<br>+             tmp = find_resource(AST_VECTOR_GET(&localdeps, i), 0);<br>+<br>+                if (!tmp) {<br>+                  goto clean_return;<br>+           }<br>+<br>+         if (AST_VECTOR_GET_CMP(missingdeps, tmp, AST_VECTOR_ELEM_DEFAULT_CMP)) {<br>+                     AST_VECTOR_REMOVE(&localdeps, i, 0);<br>+             } else {<br>+                     /* missingdeps is the real list so keep it sorted. */<br>+                        if (AST_VECTOR_ADD_SORTED(missingdeps, tmp, module_vector_cmp)) {<br>+                            goto clean_return;<br>+                   }<br>+                    i++;<br>+         }<br>+    }<br>+<br>+ res = 0;<br>+     for (i = 0; !res && i < AST_VECTOR_SIZE(&localdeps); i++) {<br>+           tmp = find_resource(AST_VECTOR_GET(&localdeps, i), 0);<br>+           /* We've already confirmed tmp is loaded in the first loop. */<br>+           res = module_deps_missing_recursive(tmp, missingdeps);<br>+       }<br>+<br>+clean_return:<br>+ AST_VECTOR_FREE(&localdeps);<br>+<br>+  return res;<br> }<br> <br> const char *ast_module_name(const struct ast_module *mod)<br>@@ -244,6 +492,18 @@<br> <br> static void module_destroy(struct ast_module *mod)<br> {<br>+     AST_VECTOR_CALLBACK_VOID(&mod->requires, ast_free);<br>+   AST_VECTOR_FREE(&mod->requires);<br>+<br>+   AST_VECTOR_CALLBACK_VOID(&mod->optional_apis, ast_free);<br>+      AST_VECTOR_FREE(&mod->optional_apis);<br>+<br>+      AST_VECTOR_CALLBACK_VOID(&mod->enhances, ast_free);<br>+   AST_VECTOR_FREE(&mod->enhances);<br>+<br>+   AST_VECTOR_CALLBACK_VOID(&mod->reffed_deps, ast_module_unref);<br>+        AST_VECTOR_FREE(&mod->reffed_deps);<br>+<br>         AST_LIST_HEAD_DESTROY(&mod->users);<br>    ao2_cleanup(mod->ref_debug);<br>       ast_free(mod);<br>@@ -267,7 +527,7 @@<br>   AST_DLLIST_TRAVERSE_BACKWARDS_SAFE_END;<br>       AST_DLLIST_UNLOCK(&module_list);<br> <br>-      if (mod) {<br>+   if (mod && !mod->usecount) {<br>               ast_debug(5, "Unregistering module %s\n", info->name);<br>           module_destroy(mod);<br>  }<br>@@ -1117,6 +1377,21 @@<br>             return AST_MODULE_LOAD_FAILURE;<br>       }<br> <br>+ if (module_deps_reference(mod, NULL)) {<br>+              struct module_vector missing;<br>+                int i;<br>+<br>+            AST_VECTOR_INIT(&missing, 0);<br>+            if (module_deps_missing_recursive(mod, &missing)) {<br>+                      ast_log(LOG_ERROR, "%s has one or more unknown dependencies.\n", mod->info->name);<br>+           }<br>+            for (i = 0; i < AST_VECTOR_SIZE(&missing); i++) {<br>+                     ast_log(LOG_ERROR, "%s loaded before dependency %s!\n", mod->info->name,<br>+                             AST_VECTOR_GET(&missing, i)->info->name);<br>+          }<br>+            AST_VECTOR_FREE(&missing);<br>+       }<br>+<br>  if (!ast_fully_booted) {<br>              ast_verb(1, "Loading %s.\n", mod->resource);<br>     }<br>@@ -1184,6 +1459,17 @@<br>                     }<br>                     return required ? AST_MODULE_LOAD_FAILURE : AST_MODULE_LOAD_DECLINE;<br>          }<br>+<br>+         /* Split lists from mod->info. */<br>+         res  = ast_vector_string_create(&mod->requires, mod->info->requires, ",", strcasecmp);<br>+                res |= ast_vector_string_create(&mod->optional_apis, mod->info->optional_apis, ",", strcasecmp);<br>+              res |= ast_vector_string_create(&mod->enhances, mod->info->enhances, ",", strcasecmp);<br>+                if (res) {<br>+                   ast_log(LOG_WARNING, "Failed to initialize dependency structures for module '%s'.\n", resource_name);<br>+                      unload_dynamic_module(mod);<br>+<br>+                       return required ? AST_MODULE_LOAD_FAILURE : AST_MODULE_LOAD_DECLINE;<br>+         }<br>     }<br> <br>  if (inspect_module(mod)) {<br>@@ -1198,6 +1484,12 @@<br>            }<br>             res = AST_MODULE_LOAD_PRIORITY;<br>       } else {<br>+             if (module_deps_reference(mod, NULL)) {<br>+                      ast_log(LOG_WARNING, "Module '%s' could not be loaded due to missing dependencies.\n", resource_name);<br>+                     unload_dynamic_module(mod);<br>+<br>+                       return required ? AST_MODULE_LOAD_FAILURE : AST_MODULE_LOAD_DECLINE;<br>+         }<br>             res = start_resource(mod);<br>    }<br> <br>@@ -1255,6 +1547,119 @@<br> }<br> <br> AST_LIST_HEAD_NOLOCK(load_retries, load_order_entry);<br>+<br>+static enum ast_module_load_result start_resource_attempt(struct ast_module *mod, int *count)<br>+{<br>+  enum ast_module_load_result lres;<br>+<br>+ /* Try to grab required references. */<br>+       if (module_deps_reference(mod, NULL)) {<br>+              /* We're likely to retry so not an error. */<br>+             ast_debug(1, "Module %s is missing dependencies\n", mod->resource);<br>+             return AST_MODULE_LOAD_SKIP;<br>+ }<br>+<br>+ lres = start_resource(mod);<br>+  ast_debug(3, "START: %-46s[%d] %d\n",<br>+              mod->resource,<br>+            ast_test_flag(mod->info, AST_MODFLAG_LOAD_ORDER) ? mod->info->load_pri : AST_MODPRI_DEFAULT,<br>+                lres);<br>+<br>+    if (lres == AST_MODULE_LOAD_SUCCESS) {<br>+               (*count)++;<br>+  } else if (lres == AST_MODULE_LOAD_FAILURE) {<br>+                ast_log(LOG_ERROR, "*** Failed to load module %s\n", mod->resource);<br>+    }<br>+<br>+ return lres;<br>+}<br>+<br>+static int start_resource_list(struct module_vector *resources, int *mod_count)<br>+{<br>+    struct module_vector missingdeps;<br>+    int res = 0;<br>+<br>+      AST_VECTOR_INIT(&missingdeps, 0);<br>+        while (AST_VECTOR_SIZE(resources)) {<br>+         struct ast_module *mod = AST_VECTOR_REMOVE(resources, 0, 1);<br>+         enum ast_module_load_result lres;<br>+<br>+         lres = start_resource_attempt(mod, mod_count);<br>+               if (lres == AST_MODULE_LOAD_SUCCESS) {<br>+                       /* No missing dependencies, successful. */<br>+                   continue;<br>+            }<br>+<br>+         if (lres == AST_MODULE_LOAD_FAILURE) {<br>+                       ast_log(LOG_ERROR, "Failed to load %s.\n", ast_module_name(mod));<br>+                  res = -1;<br>+                    break;<br>+               }<br>+<br>+         if (lres == AST_MODULE_LOAD_DECLINE) {<br>+                       /* BUGBUG: Need to exempt from enhances and optional_apis. */<br>+                        continue;<br>+            }<br>+<br>+         res = module_deps_missing_recursive(mod, &missingdeps);<br>+          if (res) {<br>+                   break;<br>+               }<br>+<br>+         if (!AST_VECTOR_SIZE(&missingdeps)) {<br>+                    ast_log(LOG_WARNING, "%s isn't missing any dependencies but still didn't start\n",<br>+                         ast_module_name(mod));<br>+                       /* Dependencies were met but the module failed to start. */<br>+                  res = -1;<br>+                    break;<br>+               }<br>+<br>+         ast_debug(1, "%s has %d dependencies\n",<br>+                   ast_module_name(mod), (int)AST_VECTOR_SIZE(&missingdeps));<br>+               while (AST_VECTOR_SIZE(&missingdeps)) {<br>+                  int didwork = 0;<br>+                     int i = 0;<br>+<br>+                        while (i < AST_VECTOR_SIZE(&missingdeps)) {<br>+                           struct ast_module *tmp = AST_VECTOR_GET(&missingdeps, i);<br>+<br>+                             ast_debug(1, "%s trying to start %s\n", ast_module_name(mod), ast_module_name(tmp));<br>+                               if (!start_resource_attempt(tmp, mod_count)) {<br>+                                       ast_debug(1, "%s started %s\n", ast_module_name(mod), ast_module_name(tmp));<br>+                                       AST_VECTOR_REMOVE(&missingdeps, i, 1);<br>+                                   AST_VECTOR_REMOVE_CMP_ORDERED(resources, tmp,<br>+                                                AST_VECTOR_ELEM_DEFAULT_CMP, AST_VECTOR_ELEM_CLEANUP_NOOP);<br>+                                  didwork++;<br>+                                   continue;<br>+                            }<br>+                            ast_debug(1, "%s failed to start %s\n", ast_module_name(mod), ast_module_name(tmp));<br>+                               i++;<br>+                 }<br>+<br>+                 if (!didwork) {<br>+                              break;<br>+                       }<br>+            }<br>+<br>+         if (AST_VECTOR_SIZE(&missingdeps)) {<br>+                     ast_log(LOG_ERROR, "Failed to load %s due to unfilled dependencies.\n",<br>+                            ast_module_name(mod));<br>+                       /* We've failed to mod because of dependencies.  Really abort? */<br>+                        res = -1;<br>+                    break;<br>+               }<br>+<br>+         res = start_resource_attempt(mod, mod_count);<br>+                if (res) {<br>+                   ast_log(LOG_ERROR, "Failed to load %s: %d\n", ast_module_name(mod), res);<br>+                  break;<br>+               }<br>+    }<br>+<br>+ AST_VECTOR_FREE(&missingdeps);<br>+<br>+        return res;<br>+}<br> <br> /*! loads modules in order by load_pri, updates mod_count<br>        \return -1 on failure to load module, -2 on failure to load required module, otherwise 0<br>@@ -1358,30 +1763,9 @@<br>              AST_LIST_TRAVERSE_SAFE_END;<br>   }<br> <br>- /* second remove modules from heap sorted by priority */<br>-     for (i = 0; i < AST_VECTOR_SIZE(&resource_heap); i++) {<br>-               struct ast_module *mod = AST_VECTOR_GET(&resource_heap, i);<br>-              enum ast_module_load_result lres;<br>-<br>-         lres = start_resource(mod);<br>-          ast_debug(3, "START: %-46s %d %d\n", mod->resource, lres, global_symbols);<br>-              switch (lres) {<br>-              case AST_MODULE_LOAD_SUCCESS:<br>-                        count++;<br>-             case AST_MODULE_LOAD_DECLINE:<br>-                        break;<br>-               case AST_MODULE_LOAD_FAILURE:<br>-                        ast_log(LOG_ERROR, "*** Failed to load module %s\n", mod->resource);<br>-                    res = -1;<br>-                    goto done;<br>-           case AST_MODULE_LOAD_SKIP:<br>-           case AST_MODULE_LOAD_PRIORITY:<br>-                       break;<br>-               }<br>-    }<br>+    res = start_resource_list(&resource_heap, &count);<br> <br> done:<br>-<br>      while ((order = AST_LIST_REMOVE_HEAD(&load_retries, entry))) {<br>            ast_free(order->resource);<br>                 ast_free(order);<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7873">change 7873</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7873"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I9be08d1dd331aceadc1dcba00b804d71360b2fbb </div>
<div style="display:none"> Gerrit-Change-Number: 7873 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>