res_pjsip_phoneprov_provider: Fix reference leak on unload

res_pjsip_phoneprov_provider was leaking references to phoneprov objects due to
a missing OBJ_NODATA in an ao2_callback in load_users().  Rather than adding the
OBJ_NODATA, I changed load_users to use a more straightforward ao2_iterator.
This plugged the leak but exposed an unload order issue between
res_pjsip_phoneprov_provider, res_phoneprov and res_pjsip.

res_pjsip_phoneprov_provider unloads first, then res_phoneprov, then res_pjsip.
Since res_pjsip_phoneprov_provider uses res_pjsip's sorcery instance, when it
unloads, it's objects are still in the sorcery instance.  When res_pjsip
unloads, it destroys all its objects including res_pjsip_phoneprov_provider's.
The phoneprov destructor then attempts to unregister the extension from
res_phoneprov but because res_phoneprov is already cleaned up, its users
container is gone and we get a FRACK.

Simple solution, check for the NULL users container before attempting to remove
the entry. Duh.

Ran tests/res_phoneprov/res_phoneprov_provider.  No leaks in
res_pjsip_phoneprov_provider and no FRACKs.

Reported-by: Corey Farrell
Tested-by: George Joseph
Review: https://reviewboard.asterisk.org/r/4608/
ASTERISK-24935 #close
........

Merged revisions 434545 from http://svn.asterisk.org/svn/asterisk/branches/13

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@434547 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/res/res_phoneprov.c b/res/res_phoneprov.c
index ccaa6d2461565c621e31a931bc9e274fd70a5476..51c03dbf9ca33779e2597e4de10e5debd69205eb 100644 (file)
--- a/res/res_phoneprov.c
+++ b/res/res_phoneprov.c
@@ -1581,12 +1581,20 @@ static int extension_delete_cb(void *obj, void *arg, void *data, int flags)
 
 void ast_phoneprov_delete_extension(char *provider_name, char *macaddress)
 {
+       if (!users) {
+               return;
+       }
+
        ao2_callback_data(users, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE | OBJ_SEARCH_KEY,
                extension_delete_cb, macaddress, provider_name);
 }
 
 void ast_phoneprov_delete_extensions(char *provider_name)
 {
+       if (!users) {
+               return;
+       }
+
        ao2_callback(users, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, extensions_delete_cb, provider_name);
 }
 

diff --git a/res/res_pjsip_phoneprov_provider.c b/res/res_pjsip_phoneprov_provider.c
index 70dd99e067bab2920a6c0e5878806e6f28bd6304..8dafabc53d267de6dab28f623955d7e574737e7c 100644 (file)
--- a/res/res_pjsip_phoneprov_provider.c
+++ b/res/res_pjsip_phoneprov_provider.c
@@ -286,9 +286,8 @@ static int load_endpoint(const char *id, const char *endpoint_name, struct varsh
 }
 
 /*! \brief Callback that validates the phoneprov object */
-static int users_apply_handler(void *obj, void *arg, int flags)
+static void users_apply_handler(struct phoneprov *pp)
 {
-       struct phoneprov *pp = obj;
        const char *id = ast_sorcery_object_get_id(pp);
        const char *endpoint_name;
        char port_string[6];
@@ -296,19 +295,19 @@ static int users_apply_handler(void *obj, void *arg, int flags)
        if (!ast_var_find(pp->vars,
                ast_phoneprov_std_variable_lookup(AST_PHONEPROV_STD_MAC))) {
                ast_log(LOG_ERROR, "phoneprov %s must contain a MAC entry.\n", id);
-               return 0;
+               return;
        }
 
        if (!ast_var_find(pp->vars,
                ast_phoneprov_std_variable_lookup(AST_PHONEPROV_STD_PROFILE))) {
                ast_log(LOG_ERROR, "phoneprov %s must contain a PROFILE entry.\n", id);
-               return 0;
+               return;
        }
 
        endpoint_name = ast_var_find(pp->vars, "endpoint");
        if (endpoint_name) {
                if (load_endpoint(id, endpoint_name, pp->vars, port_string)) {
-                       return 0;
+                       return;
                }
        }
 
@@ -337,13 +336,15 @@ static int users_apply_handler(void *obj, void *arg, int flags)
 
        ast_phoneprov_add_extension(AST_MODULE, pp->vars);
 
-       return CMP_MATCH;
+       return;
 }
 
 /*! \brief Callback that loads the users from phoneprov sections */
 static int load_users(void)
 {
        struct ao2_container *users;
+       struct ao2_iterator i;
+       struct phoneprov *pp;
 
        ast_sorcery_reload_object(sorcery, "phoneprov");
 
@@ -353,13 +354,17 @@ static int load_users(void)
                return 0;
        }
 
-       ao2_callback(users, OBJ_MULTIPLE, users_apply_handler, sorcery);
+       i = ao2_iterator_init(users, 0);
+       while ((pp = ao2_iterator_next(&i))) {
+               users_apply_handler(pp);
+               ao2_ref(pp, -1);
+       }
+       ao2_iterator_destroy(&i);
        ao2_ref(users, -1);
 
        return 0;
 }
 
-
 static int load_module(void)
 {
        CHECK_PJSIP_MODULE_LOADED();