[asterisk-commits] rmudgett: branch 10 r377354 - in /branches/10/apps: ./ confbridge/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Dec 6 17:56:48 CST 2012


Author: rmudgett
Date: Thu Dec  6 17:56:45 2012
New Revision: 377354

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=377354
Log:
confbridge: Fix some resource leaks on conference teardown.

* 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.

Modified:
    branches/10/apps/app_confbridge.c
    branches/10/apps/confbridge/conf_config_parser.c

Modified: branches/10/apps/app_confbridge.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/apps/app_confbridge.c?view=diff&rev=377354&r1=377353&r2=377354
==============================================================================
--- branches/10/apps/app_confbridge.c (original)
+++ branches/10/apps/app_confbridge.c Thu Dec  6 17:56:45 2012
@@ -296,7 +296,7 @@
 /*! \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,
@@ -560,9 +560,7 @@
 {
 	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);
@@ -768,8 +766,6 @@
 
 	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 = conference_bridge->playback_chan->tech->bridged_channel(conference_bridge->playback_chan, NULL);
 		if (underlying_channel) {
@@ -784,7 +780,11 @@
 		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
@@ -961,7 +961,7 @@
 	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),
 				"");
@@ -973,9 +973,16 @@
 		/* 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;
@@ -987,7 +994,7 @@
 			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;
 		}
 
@@ -1000,15 +1007,15 @@
 			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;
@@ -1021,7 +1028,7 @@
 		}
 
 		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);
@@ -1040,7 +1047,7 @@
 
 	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;
 	}
 
@@ -1049,7 +1056,7 @@
 	/* 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;
 		}
 	}
@@ -1057,7 +1064,7 @@
 	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;
 		}
 	}
@@ -1072,21 +1079,26 @@
 }
 
 /*!
- * \brief Leave a conference bridge
+ * \brief Leave a conference
  *
- * \param conference_bridge The conference bridge to leave
- * \param conference_bridge_user The conference bridge user structure
- *
+ * \param user The conference user
  */
-static void leave_conference_bridge(struct conference_bridge *conference_bridge, struct conference_bridge_user *conference_bridge_user)
-{
-	ao2_lock(conference_bridge);
-
-	handle_conf_user_leave(conference_bridge_user);
-
-	/* Done mucking with the conference bridge, huzzah */
-	ao2_unlock(conference_bridge);
-	ao2_ref(conference_bridge, -1);
+static void leave_conference(struct conference_bridge_user *user)
+{
+	struct post_join_action *action;
+
+	ao2_lock(user->conference_bridge);
+	handle_conf_user_leave(user);
+	ao2_unlock(user->conference_bridge);
+
+	/* 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;
 }
 
 /*!
@@ -1469,7 +1481,7 @@
 
 	/* 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;
 	}
@@ -1495,7 +1507,7 @@
 	}
 
 	/* 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 */

Modified: branches/10/apps/confbridge/conf_config_parser.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/apps/confbridge/conf_config_parser.c?view=diff&rev=377354&r1=377353&r2=377354
==============================================================================
--- branches/10/apps/confbridge/conf_config_parser.c (original)
+++ branches/10/apps/confbridge/conf_config_parser.c Thu Dec  6 17:56:45 2012
@@ -1372,7 +1372,7 @@
 
 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);
 	}




More information about the asterisk-commits mailing list