diff options
author | George Joseph <george.joseph@fairview5.com> | 2015-04-09 23:12:13 +0000 |
---|---|---|
committer | George Joseph <george.joseph@fairview5.com> | 2015-04-09 23:12:13 +0000 |
commit | ed6b6e3c03d9ab54bb3fcb2fe3c912837dd3a007 (patch) | |
tree | 3b1ba5bb70c878eb2d2c038a0693222f7ca721cf | |
parent | 9a63ada03af4a9c65819d21de727817bed93690a (diff) |
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
-rw-r--r-- | res/res_phoneprov.c | 8 | ||||
-rw-r--r-- | res/res_pjsip_phoneprov_provider.c | 21 |
2 files changed, 21 insertions, 8 deletions
diff --git a/res/res_phoneprov.c b/res/res_phoneprov.c index ccaa6d246..51c03dbf9 100644 --- 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 70dd99e06..8dafabc53 100644 --- 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(); |