res_smdi.c: Fix module ref counting and inverted test.

I think this module is so screwed up that it doesn't work anymore.  Even
with these attempts to fix things it still won't gracefully shut down.
The module refs will not go to zero to allow unloading the module.

* Fix module ref counting dealing with the SMDI interface object.  There
were several off-nominal paths that unbalanced the module ref count.  Also
the destructor freed the ao2 object itself which is bad.  Made the
smdi_read thread not hold its own ref to the SMDI interface object so when
all refs go away the destructor will stop the listener thread.

* Fixed the smdi_load() return code of 1 concerning the number of
listeners.  The test was inverted.

Change-Id: Ic288db51b58e395d6a2fc3847f77176c16988784

diff --git a/res/res_smdi.c b/res/res_smdi.c
index 8622da4a06931e7477bcf6ec3b9a69f21682422a..123283920651d944fe49e63f181b4bbbe9b2e299 100644 (file)
--- a/res/res_smdi.c
+++ b/res/res_smdi.c
@@ -226,10 +226,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;
@@ -246,9 +248,9 @@ static void smdi_interface_destroy(void *obj)
        ast_mutex_destroy(&iface->mwi_q_lock);
        ast_cond_destroy(&iface->mwi_q_cond);
 
-       free(iface);
-
-       ast_module_unref(ast_module_info->self);
+       if (mod_unref_defer) {
+               ast_module_unref(ast_module_info->self);
+       }
 }
 
 /*!
@@ -611,7 +613,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;
                        }
 
@@ -621,7 +622,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;
@@ -638,7 +638,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;
@@ -654,7 +653,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;
@@ -733,7 +731,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;
                        }
 
@@ -767,7 +764,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;
@@ -788,7 +784,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;
 }
 
@@ -985,6 +980,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);
 
@@ -1107,7 +1107,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);
@@ -1157,7 +1163,7 @@ static int smdi_load(int reload)
                return -1;
        }
 
-       if (ao2_container_count(new_ifaces)) {
+       if (!ao2_container_count(new_ifaces)) {
                res = 1;
        }