[asterisk-commits] mmichelson: branch file/bridging r172507 - /team/file/bridging/apps/

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Thu Jan 29 18:35:20 2009
New Revision: 172507

URL: http://svn.digium.com/svn-view/asterisk?view=rev&rev=172507
Log:
Fix another race condition in app_confbridge

If a marked user entered the conference while an unmarked
user was listening to the prompt telling him that he is
waiting for the leader, then badness would happen. I'll
not elaborate further. It's fixed now though.

Because I'm such a good guy, I also got rid of
extra whitespace in this commit, too.


Modified:
    team/file/bridging/apps/app_confbridge.c

Modified: team/file/bridging/apps/app_confbridge.c
URL: http://svn.digium.com/svn-view/asterisk/team/file/bridging/apps/app_confbridge.c?view=diff&rev=172507&r1=172506&r2=172507
==============================================================================
--- team/file/bridging/apps/app_confbridge.c (original)
+++ team/file/bridging/apps/app_confbridge.c Thu Jan 29 18:35:20 2009
@@ -21,7 +21,7 @@
  * \brief Conference Bridge application
  *
  * \author\verbatim Joshua Colp <jcolp at digium.com> \endverbatim
- * 
+ *
  * This is a conference bridge application utilizing the bridging core.
  * \ingroup applications
  */
@@ -92,7 +92,7 @@
                                                 <para>Quiet mode (don't play enter/leave sounds).</para>
                                         </option>
 				</optionlist>
-		      </parameter> 
+		      </parameter>
                 </syntax>
                 <description>
                         <para>Enters the user into a specified conference bridge. The user can exit the conference by hangup only.</para>
@@ -221,7 +221,7 @@
 			return;
 		}
 	}
-	
+
 	return;
 }
 
@@ -261,7 +261,7 @@
 		if (conference_bridge->markedusers >= 2) {
 			return;
 		}
-		
+
 		/* Iterate through every participant stopping MOH on them if need be */
 		AST_LIST_TRAVERSE(&conference_bridge->users_list, other_conference_bridge_user, list) {
 			if (other_conference_bridge_user == conference_bridge_user) {
@@ -287,7 +287,7 @@
 			}
 			other_conference_bridge_user->features.mute = 0;
 		}
-		
+
 	} else {
 		/* If a marked user already exists in the conference bridge we can just bail out now */
 		if (conference_bridge->markedusers) {
@@ -300,7 +300,10 @@
 			play_prompt_to_channel(conference_bridge, conference_bridge_user->chan, "conf-waitforleader");
 		}
 		/* Start music on hold if needed */
-		if (ast_test_flag(&conference_bridge_user->flags, OPTION_MUSICONHOLD)) {
+		/* We need to recheck the markedusers value here. play_prompt_to_channel unlocks the conference bridge, potentially
+		 * allowing a marked user to enter while the prompt was playing
+		 */
+		if (!conference_bridge->markedusers && ast_test_flag(&conference_bridge_user->flags, OPTION_MUSICONHOLD)) {
 			ast_moh_start(conference_bridge_user->chan, conference_bridge_user->opt_args[OPTION_MUSICONHOLD_CLASS], NULL);
 		}
 	}
@@ -369,14 +372,14 @@
 	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);
 		ast_hangup(underlying_channel);
 		ast_hangup(conference_bridge->playback_chan);
 		conference_bridge->playback_chan = NULL;
 	}
-	
+
 	/* Destroying a conference bridge is simple, all we have to do is destroy the bridging object */
 	ast_bridge_destroy(conference_bridge->bridge);
 	conference_bridge->bridge = NULL;
@@ -439,7 +442,7 @@
 
 		/* Setup lock for playback channel */
 		ast_mutex_init(&conference_bridge->playback_lock);
-		
+
 		/* Link it into the conference bridges container */
 		ao2_link(conference_bridges, conference_bridge);
 
@@ -557,17 +560,17 @@
 	struct ast_channel *underlying_channel;
 
 	ast_mutex_lock(&conference_bridge->playback_lock);
-	
+
 	if (!(conference_bridge->playback_chan)) {
 		int cause;
-		
+
 		if (!(conference_bridge->playback_chan = ast_request("Bridge", AST_FORMAT_SLINEAR, "", &cause))) {
 			ast_mutex_unlock(&conference_bridge->playback_lock);
 			return -1;
 		}
 
 		conference_bridge->playback_chan->bridge = conference_bridge->bridge;
-		
+
 		if (ast_call(conference_bridge->playback_chan, "", 0)) {
 			ast_hangup(conference_bridge->playback_chan);
 			conference_bridge->playback_chan = NULL;
@@ -591,7 +594,7 @@
 	ast_bridge_depart(conference_bridge->bridge, underlying_channel);
 
 	ast_mutex_unlock(&conference_bridge->playback_lock);
-	
+
 	return 0;
 }
 
@@ -765,7 +768,7 @@
 	/* Easy as pie, depart this channel from the conference bridge */
 	leave_conference_bridge(conference_bridge, &conference_bridge_user);
 	conference_bridge = NULL;
-	
+
 	/* Can't forget to clean up the features structure, or else we risk a memory leak */
 	ast_bridge_features_cleanup(&conference_bridge_user.features);
 
@@ -789,7 +792,7 @@
 static int unload_module(void)
 {
 	int res = ast_unregister_application(app);
-	
+
 	/* Get rid of the conference bridges container. Since we only allow dynamic ones none will be active. */
 	ao2_ref(conference_bridges, -1);