confbridge: Fix some resource leaks on conference teardown.
authorRichard Mudgett <rmudgett@digium.com>
Fri, 7 Dec 2012 00:00:39 +0000 (00:00 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Fri, 7 Dec 2012 00:00:39 +0000 (00:00 +0000)
* Made destroy_conference_bridge() destroy a missed ast_mutex_t and ast_cond_t.

* Made join_conference_bridge() init the ast_mutex_t's and ast_cond_t so
destroy_conference_bridge() can destroy them unconditionally.

* Made join_conference_bridge() abort if the new conference could not be
added to the conferences container.

* Made leave_conference() discard any post-join actions if
join_conference_bridge() had to abort early.

* Made the join_conference_bridge() diagnostic messages better describe
what happened.

* Renamed leave_conference_bridge() to leave_conference() and made it only
take a conference user pointer.  The conference pointer was redundant.

* Made conf_bridge_profile_copy() use struct copy instead of memcpy().

* No need to lock the conference in start_conf_record_thread() since all
of the callers already have it locked.
........

Merged revisions 377354 from http://svn.asterisk.org/svn/asterisk/branches/10
........

Merged revisions 377355 from http://svn.asterisk.org/svn/asterisk/branches/11

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

apps/app_confbridge.c
apps/confbridge/conf_config_parser.c

index d5eabb8..b4ec076 100644 (file)
@@ -305,7 +305,7 @@ enum {
 /*! \brief Container to hold all conference bridges in progress */
 static struct ao2_container *conference_bridges;
 
-static void leave_conference_bridge(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user);
+static void leave_conference(struct conference_bridge_user *user);
 static int play_sound_number(struct conference_bridge *conference_bridge, int say_number);
 static int execute_menu_entry(struct conference_bridge *conference_bridge,
        struct conference_bridge_user *conference_bridge_user,
@@ -573,9 +573,7 @@ static int start_conf_record_thread(struct conference_bridge *conference_bridge)
 {
        ao2_ref(conference_bridge, +1); /* give the record thread a ref */
 
-       ao2_lock(conference_bridge);
        conf_start_record(conference_bridge);
-       ao2_unlock(conference_bridge);
 
        if (ast_pthread_create_background(&conference_bridge->record_thread, NULL, record_thread, conference_bridge)) {
                ast_log(LOG_WARNING, "Failed to create recording channel for conference %s\n", conference_bridge->name);
@@ -829,8 +827,6 @@ static void destroy_conference_bridge(void *obj)
 
        ast_debug(1, "Destroying conference bridge '%s'\n", conference_bridge->name);
 
-       ast_mutex_destroy(&conference_bridge->playback_lock);
-
        if (conference_bridge->playback_chan) {
                struct ast_channel *underlying_channel = ast_channel_tech(conference_bridge->playback_chan)->bridged_channel(conference_bridge->playback_chan, NULL);
                if (underlying_channel) {
@@ -845,7 +841,11 @@ static void destroy_conference_bridge(void *obj)
                ast_bridge_destroy(conference_bridge->bridge);
                conference_bridge->bridge = NULL;
        }
+
        conf_bridge_profile_destroy(&conference_bridge->b_profile);
+       ast_cond_destroy(&conference_bridge->record_cond);
+       ast_mutex_destroy(&conference_bridge->record_lock);
+       ast_mutex_destroy(&conference_bridge->playback_lock);
 }
 
 /*! \brief Call the proper join event handler for the user for the conference bridge's current state
@@ -1022,7 +1022,7 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
        if (conference_bridge && (max_members_reached || conference_bridge->locked) && !ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_ADMIN)) {
                ao2_unlock(conference_bridges);
                ao2_ref(conference_bridge, -1);
-               ast_debug(1, "Conference bridge '%s' is locked and caller is not an admin\n", name);
+               ast_debug(1, "Conference '%s' is locked and caller is not an admin\n", name);
                ast_stream_and_wait(conference_bridge_user->chan,
                                conf_get_sound(CONF_SOUND_LOCKED, conference_bridge_user->b_profile.sounds),
                                "");
@@ -1034,10 +1034,17 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
                /* Try to allocate memory for a new conference bridge, if we fail... this won't end well. */
                if (!(conference_bridge = ao2_alloc(sizeof(*conference_bridge), destroy_conference_bridge))) {
                        ao2_unlock(conference_bridges);
-                       ast_log(LOG_ERROR, "Conference bridge '%s' does not exist.\n", name);
+                       ast_log(LOG_ERROR, "Conference '%s' could not be created.\n", name);
                        return NULL;
                }
 
+               /* Setup lock for playback channel */
+               ast_mutex_init(&conference_bridge->playback_lock);
+
+               /* Setup lock for the record channel */
+               ast_mutex_init(&conference_bridge->record_lock);
+               ast_cond_init(&conference_bridge->record_cond, NULL);
+
                /* Setup conference bridge parameters */
                conference_bridge->record_thread = AST_PTHREADT_NULL;
                ast_copy_string(conference_bridge->name, name, sizeof(conference_bridge->name));
@@ -1048,7 +1055,7 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
                        ao2_ref(conference_bridge, -1);
                        conference_bridge = NULL;
                        ao2_unlock(conference_bridges);
-                       ast_log(LOG_ERROR, "Conference bridge '%s' could not be created.\n", name);
+                       ast_log(LOG_ERROR, "Conference '%s' mixing bridge could not be created.\n", name);
                        return NULL;
                }
 
@@ -1061,15 +1068,15 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
                        ast_bridge_set_talker_src_video_mode(conference_bridge->bridge);
                }
 
-               /* Setup lock for playback channel */
-               ast_mutex_init(&conference_bridge->playback_lock);
-
-               /* Setup lock for the record channel */
-               ast_mutex_init(&conference_bridge->record_lock);
-               ast_cond_init(&conference_bridge->record_cond, NULL);
-
                /* Link it into the conference bridges container */
-               ao2_link(conference_bridges, conference_bridge);
+               if (!ao2_link(conference_bridges, conference_bridge)) {
+                       ao2_ref(conference_bridge, -1);
+                       conference_bridge = NULL;
+                       ao2_unlock(conference_bridges);
+                       ast_log(LOG_ERROR,
+                               "Conference '%s' could not be added to the conferences list.\n", name);
+                       return NULL;
+               }
 
                /* Set the initial state to EMPTY */
                conference_bridge->state = CONF_STATE_EMPTY;
@@ -1082,7 +1089,7 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
                }
 
                send_conf_start_event(conference_bridge->name);
-               ast_debug(1, "Created conference bridge '%s' and linked to container '%p'\n", name, conference_bridges);
+               ast_debug(1, "Created conference '%s' and linked to container.\n", name);
        }
 
        ao2_unlock(conference_bridges);
@@ -1101,7 +1108,7 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
 
        if (ast_check_hangup(conference_bridge_user->chan)) {
                ao2_unlock(conference_bridge);
-               leave_conference_bridge(conference_bridge, conference_bridge_user);
+               leave_conference(conference_bridge_user);
                return NULL;
        }
 
@@ -1110,7 +1117,7 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
        /* Announce number of users if need be */
        if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_ANNOUNCEUSERCOUNT)) {
                if (announce_user_count(conference_bridge, conference_bridge_user)) {
-                       leave_conference_bridge(conference_bridge, conference_bridge_user);
+                       leave_conference(conference_bridge_user);
                        return NULL;
                }
        }
@@ -1118,7 +1125,7 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
        if (ast_test_flag(&conference_bridge_user->u_profile, USER_OPT_ANNOUNCEUSERCOUNTALL) &&
                (conference_bridge->activeusers > conference_bridge_user->u_profile.announce_user_count_all_after)) {
                if (announce_user_count(conference_bridge, NULL)) {
-                       leave_conference_bridge(conference_bridge, conference_bridge_user);
+                       leave_conference(conference_bridge_user);
                        return NULL;
                }
        }
@@ -1133,21 +1140,26 @@ static struct conference_bridge *join_conference_bridge(const char *name, struct
 }
 
 /*!
- * \brief Leave a conference bridge
- *
- * \param conference_bridge The conference bridge to leave
- * \param conference_bridge_user The conference bridge user structure
+ * \brief Leave a conference
  *
+ * \param user The conference user
  */
-static void leave_conference_bridge(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user)
+static void leave_conference(struct conference_bridge_user *user)
 {
-       ao2_lock(conference_bridge);
+       struct post_join_action *action;
 
-       handle_conf_user_leave(conference_bridge_user);
+       ao2_lock(user->conference_bridge);
+       handle_conf_user_leave(user);
+       ao2_unlock(user->conference_bridge);
 
-       /* Done mucking with the conference bridge, huzzah */
-       ao2_unlock(conference_bridge);
-       ao2_ref(conference_bridge, -1);
+       /* Discard any post-join actions */
+       while ((action = AST_LIST_REMOVE_HEAD(&user->post_join_list, list))) {
+               ast_free(action);
+       }
+
+       /* Done mucking with the conference, huzzah */
+       ao2_ref(user->conference_bridge, -1);
+       user->conference_bridge = NULL;
 }
 
 /*!
@@ -1544,7 +1556,7 @@ static int confbridge_exec(struct ast_channel *chan, const char *data)
 
        /* if we're shutting down, don't attempt to do further processing */
        if (ast_shutting_down()) {
-               leave_conference_bridge(conference_bridge, &conference_bridge_user);
+               leave_conference(&conference_bridge_user);
                conference_bridge = NULL;
                goto confbridge_cleanup;
        }
@@ -1570,7 +1582,7 @@ static int confbridge_exec(struct ast_channel *chan, const char *data)
        }
 
        /* Easy as pie, depart this channel from the conference bridge */
-       leave_conference_bridge(conference_bridge, &conference_bridge_user);
+       leave_conference(&conference_bridge_user);
        conference_bridge = NULL;
 
        /* If the user was kicked from the conference play back the audio prompt for it */
index 39bf298..4868d3d 100644 (file)
@@ -1394,7 +1394,7 @@ const struct user_profile *conf_find_user_profile(struct ast_channel *chan, cons
 
 void conf_bridge_profile_copy(struct bridge_profile *dst, struct bridge_profile *src)
 {
-       memcpy(dst, src, sizeof(*dst));
+       *dst = *src;
        if (src->sounds) {
                ao2_ref(src->sounds, +1);
        }