Fix race condition for CEL LINKEDID_END event

This patch fixes to situations that could cause the CEL LINKEDID_END event to
be missed.

1) During a core stop gracefully, modules are unloaded when ast_active_channels
== 0. The LINKDEDID_END event fires during the channel destructor. This means
that occasionally, the cel_* module will be unloaded before the channel is
destroyed. It seemed generally useful to wait until the refcount of all
channels == 0 before unloading, so I added a channel counter and used it in the
shutdown code.

2) During a masquerade, ast_channel_change_linkedid is called. It calls
ast_cel_check_retire_linkedid which unrefs the linkedid in the linkedids
container in cel.c. It didn't ref the new linkedid. Now it does.

Review: https://reviewboard.asterisk.org/r/1900/
........

Merged revisions 367292 from http://svn.asterisk.org/svn/asterisk/branches/1.8

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

diff --git a/include/asterisk/cel.h b/include/asterisk/cel.h
index b388ffd9b5366c8f5eeecb8c1febe3fdce29e0c3..25a3a4af1d4773003cbe80db2fb5311a22375622 100644 (file)
--- a/include/asterisk/cel.h
+++ b/include/asterisk/cel.h
@@ -187,6 +187,15 @@ const char *ast_cel_get_ama_flag_name(enum ast_cel_ama_flag flag);
  */
 void ast_cel_check_retire_linkedid(struct ast_channel *chan);
 
+/*!
+ * \brief Inform CEL that a new linkedid is being used
+ * \since 11
+ *
+ * \retval -1 error
+ * \retval 0 success
+ */
+int ast_cel_linkedid_ref(const char *linkedid);
+
 /*!
  * \brief Create a fake channel from data in a CEL event
  *

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 8a371a11f3c03ba6d455f51d250463adc3cb4bbf..4e16b5ae7d8f0da77c9cc2fa667db74b5b7ee1cd 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -2166,9 +2166,12 @@ void ast_begin_shutdown(int hangup);
 /*! Cancels an existing shutdown and returns to normal operation */
 void ast_cancel_shutdown(void);
 
-/*! \return number of active/allocated channels */
+/*! \return number of channels available for lookup */
 int ast_active_channels(void);
 
+/*! \return the number of channels not yet destroyed */
+int ast_undestroyed_channels(void);
+
 /*! \return non-zero if Asterisk is being shut down */
 int ast_shutting_down(void);
 

diff --git a/main/asterisk.c b/main/asterisk.c
index ac30958c3171c28e97c558aa784e2d6c8e48d18f..3ebcebe540a5cb1acc3514f5153203c72dabec37 100644 (file)
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -1701,7 +1701,7 @@ static int can_safely_quit(shutdown_nice_t niceness, int restart)
                for (;;) {
                        time(&e);
                        /* Wait up to 15 seconds for all channels to go away */
-                       if ((e - s) > 15 || !ast_active_channels() || shuttingdown != niceness) {
+                       if ((e - s) > 15 || !ast_undestroyed_channels() || shuttingdown != niceness) {
                                break;
                        }
                        /* Sleep 1/10 of a second */
@@ -1715,7 +1715,7 @@ static int can_safely_quit(shutdown_nice_t niceness, int restart)
                        ast_verbose("Waiting for inactivity to perform %s...\n", restart ? "restart" : "halt");
                }
                for (;;) {
-                       if (!ast_active_channels() || shuttingdown != niceness) {
+                       if (!ast_undestroyed_channels() || shuttingdown != niceness) {
                                break;
                        }
                        sleep(1);

diff --git a/main/cel.c b/main/cel.c
index 023fd571a9a6c7b7b6fa3ebc83b98d671f1d1953..ce9a15f97d10172fb410a2394fee1afb381ec980 100644 (file)
--- a/main/cel.c
+++ b/main/cel.c
@@ -464,6 +464,31 @@ struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event
        return tchan;
 }
 
+int ast_cel_linkedid_ref(const char *linkedid)
+{
+       char *lid;
+
+       if (ast_strlen_zero(linkedid)) {
+               ast_log(LOG_ERROR, "The linkedid should never be empty\n");
+               return -1;
+       }
+
+       if (!(lid = ao2_find(linkedids, (void *) linkedid, OBJ_POINTER))) {
+               if (!(lid = ao2_alloc(strlen(linkedid) + 1, NULL))) {
+                       return -1;
+               }
+               strcpy(lid, linkedid);
+               if (!ao2_link(linkedids, lid)) {
+                       ao2_ref(lid, -1);
+                       return -1;
+               }
+               /* Leave both the link and the alloc refs to show a count of 1 + the link */
+       }
+       /* If we've found, go ahead and keep the ref to increment count of how many channels
+        * have this linkedid. We'll clean it up in check_retire */
+       return 0;
+}
+
 int ast_cel_report_event(struct ast_channel *chan, enum ast_cel_event_type event_type,
                const char *userdefevname, const char *extra, struct ast_channel *peer2)
 {
@@ -480,22 +505,10 @@ int ast_cel_report_event(struct ast_channel *chan, enum ast_cel_event_type event
        /* Record the linkedid of new channels if we are tracking LINKEDID_END even if we aren't
         * reporting on CHANNEL_START so we can track when to send LINKEDID_END */
        if (cel_enabled && ast_cel_track_event(AST_CEL_LINKEDID_END) && event_type == AST_CEL_CHANNEL_START && chan->linkedid) {
-               char *lid;
-               if (!(lid = ao2_find(linkedids, (void *) chan->linkedid, OBJ_POINTER))) {
-                       if (!(lid = ao2_alloc(strlen(chan->linkedid) + 1, NULL))) {
-                               ast_mutex_unlock(&reload_lock);
-                               return -1;
-                       }
-                       strcpy(lid, chan->linkedid);
-                       if (!ao2_link(linkedids, lid)) {
-                               ao2_ref(lid, -1);
-                               ast_mutex_unlock(&reload_lock);
-                               return -1;
-                       }
-                       /* Leave both the link and the alloc refs to show a count of 1 + the link */
+               if (ast_cel_linkedid_ref(chan->linkedid)) {
+                       ast_mutex_unlock(&reload_lock);
+                       return -1;
                }
-               /* If we've found, go ahead and keep the ref to increment count of how many channels
-                * have this linkedid. We'll clean it up in check_retire */
        }
 
        if (!cel_enabled || !ast_cel_track_event(event_type)) {

diff --git a/main/channel.c b/main/channel.c
index 2bb4a20e6af5a722b7051be4183a2e99c311af5a..b3a51bafd79ccdbbeaf5cdbab53ccaa2ffaeb47b 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -93,6 +93,7 @@ struct ast_epoll_data {
 static int shutting_down;
 
 static int uniqueint;
+static int chancount;
 
 unsigned long global_fin, global_fout;
 
@@ -833,6 +834,11 @@ int ast_active_channels(void)
        return channels ? ao2_container_count(channels) : 0;
 }
 
+int ast_undestroyed_channels(void)
+{
+       return ast_atomic_fetchadd_int(&chancount, 0);
+}
+
 /*! \brief Cancel a shutdown in progress */
 void ast_cancel_shutdown(void)
 {
@@ -1310,6 +1316,7 @@ __ast_channel_alloc_ap(int needqueue, int state, const char *cid_num, const char
        ast_cdr_init(tmp->cdr, tmp);
        ast_cdr_start(tmp->cdr);
 
+       ast_atomic_fetchadd_int(&chancount, +1);
        ast_cel_report_event(tmp, AST_CEL_CHANNEL_START, NULL, NULL, NULL);
 
        headp = &tmp->varshead;
@@ -2507,6 +2514,7 @@ static void ast_channel_destructor(void *obj)
        }
 
        chan->nativeformats = ast_format_cap_destroy(chan->nativeformats);
+       ast_atomic_fetchadd_int(&chancount, -1);
 }
 
 /*! \brief Free a dummy channel structure */
@@ -6460,15 +6468,17 @@ static const char *oldest_linkedid(const char *a, const char *b)
  *  see if the channel's old linkedid is now being retired */
 static void ast_channel_change_linkedid(struct ast_channel *chan, const char *linkedid)
 {
+       ast_assert(linkedid != NULL);
        /* if the linkedid for this channel is being changed from something, check... */
-       if (!ast_strlen_zero(chan->linkedid) && 0 != strcmp(chan->linkedid, linkedid)) {
-               ast_cel_check_retire_linkedid(chan);
+       if (!strcmp(chan->linkedid, linkedid)) {
+               return;
        }
 
+       ast_cel_check_retire_linkedid(chan);
        ast_string_field_set(chan, linkedid, linkedid);
+       ast_cel_linkedid_ref(linkedid);
 }
 
-
 /*!
   \brief Propagate the oldest linkedid between associated channels