Merge "func_periodic_hook.c: Cleanup module resources on failure."
authorGeorge Joseph <gjoseph@digium.com>
Fri, 5 Oct 2018 19:32:21 +0000 (14:32 -0500)
committerGerrit Code Review <gerrit2@gerrit.digium.api>
Fri, 5 Oct 2018 19:32:21 +0000 (14:32 -0500)
codecs/codec_speex.c
res/res_smdi.c
res/res_statsd.c

index 591fce9..0354bd5 100644 (file)
@@ -689,39 +689,37 @@ static int reload(void)
 
 static int unload_module(void)
 {
-       int res = 0;
-
-       res |= ast_unregister_translator(&speextolin);
-       res |= ast_unregister_translator(&lintospeex);
-       res |= ast_unregister_translator(&speexwbtolin16);
-       res |= ast_unregister_translator(&lin16tospeexwb);
-       res |= ast_unregister_translator(&speexuwbtolin32);
-       res |= ast_unregister_translator(&lin32tospeexuwb);
+       ast_unregister_translator(&speextolin);
+       ast_unregister_translator(&lintospeex);
+       ast_unregister_translator(&speexwbtolin16);
+       ast_unregister_translator(&lin16tospeexwb);
+       ast_unregister_translator(&speexuwbtolin32);
+       ast_unregister_translator(&lin32tospeexuwb);
 
-
-       return res;
+       return 0;
 }
 
 static int load_module(void)
 {
        int res = 0;
 
-       if (parse_config(0))
+       if (parse_config(0)) {
                return AST_MODULE_LOAD_DECLINE;
+       }
 
+       /* XXX It is most likely a bug in this module if we fail to register a translator */
        res |= ast_register_translator(&speextolin);
        res |= ast_register_translator(&lintospeex);
        res |= ast_register_translator(&speexwbtolin16);
        res |= ast_register_translator(&lin16tospeexwb);
        res |= ast_register_translator(&speexuwbtolin32);
        res |= ast_register_translator(&lin32tospeexuwb);
-
        if (res) {
                unload_module();
-               return res;
+               return AST_MODULE_LOAD_DECLINE;
        }
 
-       return res;
+       return AST_MODULE_LOAD_SUCCESS;
 }
 
 AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT, "Speex Coder/Decoder",
index 1d4826a..c6f3f3a 100644 (file)
  ***/
 
 static const char config_file[] = "smdi.conf";
-static int smdi_loaded;
 
 struct ast_smdi_interface {
        char name[SMDI_MAX_FILENAME_LEN];
@@ -224,10 +223,12 @@ static struct {
 static void smdi_interface_destroy(void *obj)
 {
        struct ast_smdi_interface *iface = obj;
+       int mod_unref_defer = 0;
 
        if (iface->thread != AST_PTHREADT_NULL && iface->thread != AST_PTHREADT_STOP) {
                pthread_cancel(iface->thread);
                pthread_join(iface->thread, NULL);
+               mod_unref_defer = 1;
        }
 
        iface->thread = AST_PTHREADT_STOP;
@@ -244,9 +245,9 @@ static void smdi_interface_destroy(void *obj)
        ast_mutex_destroy(&iface->mwi_q_lock);
        ast_cond_destroy(&iface->mwi_q_cond);
 
-       ast_free(iface);
-
-       ast_module_unref(ast_module_info->self);
+       if (mod_unref_defer) {
+               ast_module_unref(ast_module_info->self);
+       }
 }
 
 /*!
@@ -609,7 +610,6 @@ static void *smdi_read(void *iface_p)
 
                        md_msg = ao2_alloc(sizeof(*md_msg), NULL);
                        if (!md_msg) {
-                               ao2_ref(iface, -1);
                                return NULL;
                        }
 
@@ -619,7 +619,6 @@ static void *smdi_read(void *iface_p)
                                if (c == EOF) {
                                        ast_log(LOG_ERROR, "Unexpected EOF while reading MD message\n");
                                        ao2_ref(md_msg, -1);
-                                       ao2_ref(iface, -1);
                                        return NULL;
                                }
                                md_msg->mesg_desk_num[i] = (char) c;
@@ -636,7 +635,6 @@ static void *smdi_read(void *iface_p)
                                if (c == EOF) {
                                        ast_log(LOG_ERROR, "Unexpected EOF while reading SMDI message\n");
                                        ao2_ref(md_msg, -1);
-                                       ao2_ref(iface, -1);
                                        return NULL;
                                }
                                md_msg->mesg_desk_term[i] = (char) c;
@@ -652,7 +650,6 @@ static void *smdi_read(void *iface_p)
                        if (c == EOF) {
                                ast_log(LOG_ERROR, "Unexpected EOF while reading SMDI message\n");
                                ao2_ref(md_msg, -1);
-                               ao2_ref(iface, -1);
                                return NULL;
                        }
                        md_msg->type = (char) c;
@@ -731,7 +728,6 @@ static void *smdi_read(void *iface_p)
 
                        mwi_msg = ao2_alloc(sizeof(*mwi_msg), NULL);
                        if (!mwi_msg) {
-                               ao2_ref(iface, -1);
                                return NULL;
                        }
 
@@ -765,7 +761,6 @@ static void *smdi_read(void *iface_p)
                                if (c == EOF) {
                                        ast_log(LOG_ERROR, "Unexpected EOF while reading MWI message\n");
                                        ao2_ref(mwi_msg, -1);
-                                       ao2_ref(iface, -1);
                                        return NULL;
                                }
                                mwi_msg->cause[i] = (char) c;
@@ -786,7 +781,6 @@ static void *smdi_read(void *iface_p)
        }
 
        ast_log(LOG_ERROR, "Error reading from SMDI interface %s, stopping listener thread\n", iface->name);
-       ao2_ref(iface, -1);
        return NULL;
 }
 
@@ -983,6 +977,11 @@ static int smdi_load(int reload)
                return 0;
 
        new_ifaces = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_MUTEX, 0, NULL, smdi_ifaces_cmp_fn);
+       if (!new_ifaces) {
+               ast_config_destroy(conf);
+               return -1;
+       }
+
        for (v = ast_variable_browse(conf, "interfaces"); v; v = v->next) {
                RAII_VAR(struct ast_smdi_interface *, iface, NULL, ao2_cleanup);
 
@@ -1035,7 +1034,7 @@ static int smdi_load(int reload)
                } else if (!strcasecmp(v->name, "twostopbits")) {
                        stopbits = ast_true(v->name);
                } else if (!strcasecmp(v->name, "smdiport")) {
-                       if (reload) {
+                       if (reload && old_ifaces) {
                                /* we are reloading, check if we are already
                                 * monitoring this interface, if we are we do
                                 * not want to start it again.  This also has
@@ -1105,7 +1104,13 @@ static int smdi_load(int reload)
                        /* set the message expiry time */
                        iface->msg_expiry = msg_expiry;
 
-                       /* start the listener thread */
+                       /*
+                        * start the listener thread
+                        *
+                        * The listener thread does not actually hold a ref to iface.  When all
+                        * external refs go away, the destructor will stop the listener thread
+                        * before actually destroying the iface object.
+                        */
                        ast_verb(3, "Starting SMDI monitor thread for %s\n", iface->name);
                        if (ast_pthread_create_background(&iface->thread, NULL, smdi_read, iface)) {
                                ast_log(LOG_ERROR, "Error starting SMDI monitor thread for %s\n", iface->name);
@@ -1155,7 +1160,7 @@ static int smdi_load(int reload)
                return -1;
        }
 
-       if (ao2_container_count(new_ifaces)) {
+       if (!ao2_container_count(new_ifaces)) {
                res = 1;
        }
 
@@ -1371,7 +1376,26 @@ static struct ast_custom_function smdi_msg_function = {
        .read = smdi_msg_read,
 };
 
-static int _unload_module(int fromload);
+static int unload_module(void)
+{
+       ao2_global_obj_release(smdi_ifaces);
+
+       destroy_all_mailbox_mappings();
+
+       ast_mutex_lock(&mwi_monitor.lock);
+       mwi_monitor.stop = 1;
+       ast_cond_signal(&mwi_monitor.cond);
+       ast_mutex_unlock(&mwi_monitor.lock);
+
+       if (mwi_monitor.thread != AST_PTHREADT_NULL) {
+               pthread_join(mwi_monitor.thread, NULL);
+       }
+
+       ast_custom_function_unregister(&smdi_msg_retrieve_function);
+       ast_custom_function_unregister(&smdi_msg_function);
+
+       return 0;
+}
 
 /*!
  * \brief Load the module
@@ -1386,7 +1410,6 @@ static int _unload_module(int fromload);
 static int load_module(void)
 {
        int res;
-       smdi_loaded = 1;
 
        ast_mutex_init(&mwi_monitor.lock);
        ast_cond_init(&mwi_monitor.cond, NULL);
@@ -1394,12 +1417,10 @@ static int load_module(void)
        /* load the config and start the listener threads*/
        res = smdi_load(0);
        if (res < 0) {
-               _unload_module(1);
+               unload_module();
                return AST_MODULE_LOAD_DECLINE;
        } else if (res == 1) {
-               _unload_module(1);
                ast_log(LOG_NOTICE, "No SMDI interfaces are available to listen on, not starting SMDI listener.\n");
-               return AST_MODULE_LOAD_DECLINE;
        }
 
        ast_custom_function_register(&smdi_msg_retrieve_function);
@@ -1408,53 +1429,17 @@ static int load_module(void)
        return AST_MODULE_LOAD_SUCCESS;
 }
 
-static int _unload_module(int fromload)
-{
-       if (!smdi_loaded) {
-               return 0;
-       }
-
-       ao2_global_obj_release(smdi_ifaces);
-
-       destroy_all_mailbox_mappings();
-
-       ast_mutex_lock(&mwi_monitor.lock);
-       mwi_monitor.stop = 1;
-       ast_cond_signal(&mwi_monitor.cond);
-       ast_mutex_unlock(&mwi_monitor.lock);
-
-       if (mwi_monitor.thread != AST_PTHREADT_NULL) {
-               pthread_join(mwi_monitor.thread, NULL);
-       }
-
-       if (!fromload) {
-               ast_custom_function_unregister(&smdi_msg_retrieve_function);
-               ast_custom_function_unregister(&smdi_msg_function);
-       }
-
-       smdi_loaded = 0;
-
-       return 0;
-}
-
-static int unload_module(void)
-{
-       return _unload_module(0);
-}
-
 static int reload(void)
 {
        int res;
 
        res = smdi_load(1);
-
        if (res < 0) {
                return res;
        } else if (res == 1) {
                ast_log(LOG_WARNING, "No SMDI interfaces were specified to listen on, not starting SDMI listener.\n");
-               return 0;
-       } else
-               return 0;
+       }
+       return 0;
 }
 
 AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_GLOBAL_SYMBOLS | AST_MODFLAG_LOAD_ORDER, "Simplified Message Desk Interface (SMDI) Resource",
index 3e08152..3e52c21 100644 (file)
@@ -339,9 +339,25 @@ static int load_module(void)
                "", OPT_CHAR_ARRAY_T, 0,
                CHARFLDSET(struct conf_global_options, prefix));
 
-       if (aco_process_config(&cfg_info, 0)) {
-               aco_info_destroy(&cfg_info);
-               return AST_MODULE_LOAD_DECLINE;
+       if (aco_process_config(&cfg_info, 0) == ACO_PROCESS_ERROR) {
+               struct conf *cfg;
+
+               ast_log(LOG_NOTICE, "Could not load statsd config; using defaults\n");
+               cfg = conf_alloc();
+               if (!cfg) {
+                       aco_info_destroy(&cfg_info);
+                       return AST_MODULE_LOAD_DECLINE;
+               }
+
+               if (aco_set_defaults(&global_option, "general", cfg->global)) {
+                       ast_log(LOG_ERROR, "Failed to initialize statsd defaults.\n");
+                       ao2_ref(cfg, -1);
+                       aco_info_destroy(&cfg_info);
+                       return AST_MODULE_LOAD_DECLINE;
+               }
+
+               ao2_global_obj_replace_unref(confs, cfg);
+               ao2_ref(cfg, -1);
        }
 
        if (!is_enabled()) {