[asterisk-commits] mmichelson: branch 12 r410673 - in /branches/12: bridges/ configs/ funcs/ inc...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Mar 17 11:52:20 CDT 2014


Author: mmichelson
Date: Mon Mar 17 11:52:12 2014
New Revision: 410673

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=410673
Log:
Fix stuck channel in ARI through the introduction of synchronous bridge actions.

Playing back a file to a channel in an ARI bridge would attempt to wait until
the playback concluded before returning. The method used involved signaling the
waiting thread in the ARI custom playback function.

The problem with this is that there were some corner cases that were not accounted for:
* If a bridge channel could not be found, then we never would attempt the playback but
  would still attempt to wait for the playback to complete.
* If the bridge playfile action failed to queue, we would still attempt to wait for the
  playback to complete.
* If the bridge playfile action were queued but some circumstance caused the playback
  not to occur (the bridge dies, the channel is removed from the bridge), then we would
  never be notified.

The solution to this is to move the waiting logic into the bridge code. A new bridge
API function is added to queue a synchronous action on a bridge. The waiting thread
is notified when the queued frame has been freed, either due to an error occurring
or due to successful playback. As a failsafe, the waiting thread has a 10 minute
timeout just in case there is a frame leak somewhere.

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


Modified:
    branches/12/bridges/bridge_softmix.c
    branches/12/configs/sorcery.conf.sample
    branches/12/funcs/func_frame_trace.c
    branches/12/include/asterisk/bridge_channel.h
    branches/12/include/asterisk/frame.h
    branches/12/include/asterisk/sorcery.h
    branches/12/main/bridge_channel.c
    branches/12/main/channel.c
    branches/12/main/frame.c
    branches/12/main/sorcery.c
    branches/12/res/res_mwi_external.c
    branches/12/res/res_pjsip/config_system.c
    branches/12/res/res_pjsip/pjsip_configuration.c
    branches/12/res/res_stasis_playback.c
    branches/12/tests/test_sorcery.c
    branches/12/tests/test_sorcery_astdb.c
    branches/12/tests/test_sorcery_realtime.c

Modified: branches/12/bridges/bridge_softmix.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/bridges/bridge_softmix.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/bridges/bridge_softmix.c (original)
+++ branches/12/bridges/bridge_softmix.c Mon Mar 17 11:52:12 2014
@@ -661,6 +661,9 @@
 	case AST_FRAME_BRIDGE_ACTION:
 		res = ast_bridge_queue_everyone_else(bridge, bridge_channel, frame);
 		break;
+	case AST_FRAME_BRIDGE_ACTION_SYNC:
+		ast_log(LOG_ERROR, "Synchronous bridge action written to a softmix bridge.\n");
+		ast_assert(0);
 	default:
 		ast_debug(3, "Frame type %d unsupported\n", frame->frametype);
 		/* "Accept" the frame and discard it. */

Modified: branches/12/configs/sorcery.conf.sample
URL: http://svnview.digium.com/svn/asterisk/branches/12/configs/sorcery.conf.sample?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/configs/sorcery.conf.sample (original)
+++ branches/12/configs/sorcery.conf.sample Mon Mar 17 11:52:12 2014
@@ -41,7 +41,7 @@
 ;
 ; The following object mappings are used by the unit test to test certain functionality of sorcery.
 ;
-[test_sorcery]
+[test_sorcery_section]
 test=memory
 
 [test_sorcery_cache]

Modified: branches/12/funcs/func_frame_trace.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/funcs/func_frame_trace.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/funcs/func_frame_trace.c (original)
+++ branches/12/funcs/func_frame_trace.c Mon Mar 17 11:52:12 2014
@@ -392,6 +392,10 @@
 		ast_verbose("FrameType: Bridge\n");
 		ast_verbose("SubClass: %d\n", frame->subclass.integer);
 		break;
+	case AST_FRAME_BRIDGE_ACTION_SYNC:
+		ast_verbose("Frametype: Synchronous Bridge\n");
+		ast_verbose("Subclass: %d\n", frame->subclass.integer);
+		break;
 	}
 
 	ast_verbose("Src: %s\n", ast_strlen_zero(frame->src) ? "NOT PRESENT" : frame->src);

Modified: branches/12/include/asterisk/bridge_channel.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/include/asterisk/bridge_channel.h?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/include/asterisk/bridge_channel.h (original)
+++ branches/12/include/asterisk/bridge_channel.h Mon Mar 17 11:52:12 2014
@@ -529,6 +529,28 @@
 int ast_bridge_channel_queue_playfile(struct ast_bridge_channel *bridge_channel, ast_bridge_custom_play_fn custom_play, const char *playfile, const char *moh_class);
 
 /*!
+ * \brief Synchronously queue a bridge action play file frame onto the bridge channel.
+ * \since 12.2.0
+ *
+ * \param bridge_channel Which channel to put the frame onto.
+ * \param custom_play Call this function to play the playfile. (NULL if normal sound file to play)
+ * \param playfile Sound filename to play.
+ * \param moh_class MOH class to request bridge peers to hear while file is played.
+ *     NULL if no MOH.
+ *     Empty if default MOH class.
+ *
+ * This function will block until the queued frame has been destroyed. This will happen
+ * either if an error occurs or if the queued playback finishes.
+ *
+ * \note No locks may be held when calling this function.
+ *
+ * \retval 0 The playback was successfully queued.
+ * \retval -1 The playback could not be queued.
+ */
+int ast_bridge_channel_queue_playfile_sync(struct ast_bridge_channel *bridge_channel,
+		ast_bridge_custom_play_fn custom_play, const char *playfile, const char *moh_class);
+
+/*!
  * \brief Custom callback run on a bridge channel.
  *
  * \param bridge_channel Which channel to operate on.

Modified: branches/12/include/asterisk/frame.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/include/asterisk/frame.h?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/include/asterisk/frame.h (original)
+++ branches/12/include/asterisk/frame.h Mon Mar 17 11:52:12 2014
@@ -122,6 +122,12 @@
 	AST_FRAME_DTMF_BEGIN,
 	/*! Internal bridge module action. */
 	AST_FRAME_BRIDGE_ACTION,
+	/*! Internal synchronous bridge module action.
+	 * Synchronous bridge actions may be queued onto bridge
+	 * channels, but they absolutely must not ever be written
+	 * directly into bridges.
+	 */
+	AST_FRAME_BRIDGE_ACTION_SYNC,
 };
 #define AST_FRAME_DTMF AST_FRAME_DTMF_END
 

Modified: branches/12/include/asterisk/sorcery.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/include/asterisk/sorcery.h?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/include/asterisk/sorcery.h (original)
+++ branches/12/include/asterisk/sorcery.h Mon Mar 17 11:52:12 2014
@@ -39,7 +39,11 @@
  * object types to their respective wizards (object storage modules). If the developer would like
  * to allow the user to configure this using the sorcery.conf configuration file the
  * \ref ast_sorcery_apply_config API call can be used to read in the configuration file and apply the
- * mappings. If the storage of the object types are such that a default wizard can be used this can
+ * mappings. \ref ast_sorcery_open will automatically call \ref ast_sorcery_apply_config to allow
+ * for configuration of objects using the same category name as the module that is opening the
+ * sorcery instance. Direct calls to \ref ast_sorcery_apply_config should only be performed if a
+ * module wishes to allow for additional configuration sections in sorcery.conf to be used.
+ * If the storage of the object types are such that a default wizard can be used this can
  * be applied using the \ref ast_sorcery_apply_default API call. Note that the default mappings will not
  * override configured mappings. They are only used in the case where no configured mapping exists.
  *
@@ -322,6 +326,9 @@
  *
  * \param module The module name (AST_MODULE)
  *
+ * When called, this will automatically also call __ast_sorcery_apply_config()
+ * with the module name as the configuration section.
+ *
  * \retval non-NULL success
  * \retval NULL if allocation failed
  */
@@ -343,6 +350,17 @@
  */
 struct ast_sorcery *ast_sorcery_retrieve_by_module_name(const char *module);
 
+enum ast_sorcery_apply_result {
+	/*! Sorcery wizard failed to apply. */
+	AST_SORCERY_APPLY_FAIL = -1,
+	/*! Sorcery wizard applied successfully. */
+	AST_SORCERY_APPLY_SUCCESS = 0,
+	/*! Sorcery wizard has already been applied to the object type. */
+	AST_SORCERY_APPLY_DUPLICATE = 1,
+	/*! Default sorcery wizard is unnecessary since a wizard has already been applied to the object type. */
+	AST_SORCERY_APPLY_DEFAULT_UNNECESSARY = 2,
+};
+
 /*!
  * \brief Apply configured wizard mappings
  *
@@ -350,10 +368,17 @@
  * \param name Name of the category to use within the configuration file, normally the module name
  * \param module The module name (AST_MODULE)
  *
- * \retval 0 success
- * \retval -1 failure
- */
-int __ast_sorcery_apply_config(struct ast_sorcery *sorcery, const char *name, const char *module);
+ * This function is called automatically by __ast_sorcery_open() using the module name as the
+ * configuration category. The only reason you should call this function is if your module
+ * wishes to apply configuration from additional sections of sorcery.conf.
+ *
+ * If a configuration section attempts to apply the same sorcery wizard to an object type
+ * more than once, the wizard will only be applied one time.
+ *
+ * \return What happened when attempting to apply the default.
+ */
+enum ast_sorcery_apply_result __ast_sorcery_apply_config(struct ast_sorcery *sorcery,
+		const char *name, const char *module);
 
 #define ast_sorcery_apply_config(sorcery, name) \
 	__ast_sorcery_apply_config((sorcery), (name), AST_MODULE)
@@ -367,14 +392,14 @@
  * \param name Name of the wizard to use
  * \param data Data to be passed to wizard
  *
- * \retval 0 success
- * \retval -1 failure
+ * \return What occurred when applying the default
  *
  * \note This should be called *after* applying configuration sourced mappings
  *
  * \note Only a single default can exist per object type
  */
-int __ast_sorcery_apply_default(struct ast_sorcery *sorcery, const char *type, const char *module, const char *name, const char *data);
+enum ast_sorcery_apply_result __ast_sorcery_apply_default(struct ast_sorcery *sorcery,
+		const char *type, const char *module, const char *name, const char *data);
 
 #define ast_sorcery_apply_default(sorcery, type, name, data) \
 	__ast_sorcery_apply_default((sorcery), (type), AST_MODULE, (name), (data))

Modified: branches/12/main/bridge_channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/main/bridge_channel.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/main/bridge_channel.c (original)
+++ branches/12/main/bridge_channel.c Mon Mar 17 11:52:12 2014
@@ -35,6 +35,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include <signal.h>
+#include <semaphore.h>
 
 #include "asterisk/heap.h"
 #include "asterisk/astobj2.h"
@@ -70,6 +71,142 @@
  */
 typedef int (*ast_bridge_channel_post_action_data)(struct ast_bridge_channel *bridge_channel, enum bridge_channel_action_type action, const void *data, size_t datalen);
 
+/*!
+ * \brief Counter used for assigning synchronous bridge action IDs
+ */
+static int sync_ids;
+
+/*!
+ * \brief Frame payload for synchronous bridge actions.
+ *
+ * The payload serves as a wrapper around the actual payload of the
+ * frame, with the addition of an id used to find the associated
+ * bridge_sync object.
+ */
+struct sync_payload {
+	/*! Unique ID for this synchronous action */
+	unsigned int id;
+	/*! Actual frame data to process */
+	unsigned char data[0];
+};
+
+/*!
+ * \brief Synchronous bridge action object.
+ *
+ * Synchronous bridge actions require the ability for one thread to wait
+ * and for another thread to indicate that the action has completed. This
+ * structure facilitates that goal by providing synchronization structures.
+ */
+struct bridge_sync {
+	/*! Unique ID of this synchronization object. Corresponds with ID in synchronous frame payload */
+	unsigned int id;
+	/*! Semaphore used for synchronization */
+	sem_t sem;
+	/*! Pointer to next entry in the list */
+	AST_LIST_ENTRY(bridge_sync) list;
+};
+
+/*!
+ * \brief List holding active synchronous action objects.
+ */
+static AST_RWLIST_HEAD_STATIC(sync_structs, bridge_sync);
+
+/*!
+ * \brief initialize a synchronous bridge object.
+ *
+ * This both initializes the structure and adds it to the list of
+ * synchronization structures.
+ *
+ * \param sync_struct The synchronization object to initialize.
+ * \param id ID to assign to the synchronization object.
+ */
+static void bridge_sync_init(struct bridge_sync *sync_struct, unsigned int id)
+{
+	memset(sync_struct, 0, sizeof(*sync_struct));
+	sync_struct->id = id;
+	sem_init(&sync_struct->sem, 0, 0);
+
+	AST_RWLIST_WRLOCK(&sync_structs);
+	AST_RWLIST_INSERT_TAIL(&sync_structs, sync_struct, list);
+	AST_RWLIST_UNLOCK(&sync_structs);
+}
+
+/*!
+ * \brief Clean up a syncrhonization bridge object.
+ *
+ * This frees fields within the synchronization object and removes
+ * it from the list of active synchronization objects.
+ *
+ * Since synchronization objects are stack-allocated, it is vital
+ * that this is called before the synchronization object goes
+ * out of scope.
+ *
+ * \param sync_struct Synchronization object to clean up.
+ */
+static void bridge_sync_cleanup(struct bridge_sync *sync_struct)
+{
+	struct bridge_sync *iter;
+
+	AST_RWLIST_WRLOCK(&sync_structs);
+	AST_LIST_TRAVERSE_SAFE_BEGIN(&sync_structs, iter, list) {
+		if (iter->id == sync_struct->id) {
+			AST_LIST_REMOVE_CURRENT(list);
+			break;
+		}
+	}
+	AST_LIST_TRAVERSE_SAFE_END;
+	AST_RWLIST_UNLOCK(&sync_structs);
+
+	sem_destroy(&sync_struct->sem);
+}
+
+/*!
+ * \brief Failsafe for synchronous bridge action waiting.
+ *
+ * When waiting for a synchronous bridge action to complete,
+ * if there is a frame resource leak somewhere, it is possible
+ * that we will never get notified that the synchronous action
+ * completed.
+ *
+ * If a significant amount of time passes, then we will abandon
+ * waiting for the synchrnous bridge action to complete.
+ *
+ * This constant represents the number of milliseconds we will
+ * wait for the bridge action to complete.
+ */
+#define PLAYBACK_TIMEOUT (600 * 1000)
+
+/*!
+ * \brief Wait for a synchronous bridge action to complete.
+ *
+ * \param sync_struct Synchronization object corresponding to the bridge action.
+ */
+static void bridge_sync_wait(struct bridge_sync *sync_struct)
+{
+	struct timeval timeout_val = ast_tvadd(ast_tvnow(), ast_samp2tv(PLAYBACK_TIMEOUT, 1000));
+	struct timespec timeout_spec = {
+		.tv_sec = timeout_val.tv_sec,
+		.tv_nsec = timeout_val.tv_usec * 1000,
+	};
+
+	sem_timedwait(&sync_struct->sem, &timeout_spec);
+}
+
+/*!
+ * \brief Signal that waiting for a synchronous bridge action is no longer necessary.
+ *
+ * This may occur for several reasons
+ * \li The synchronous bridge action has completed.
+ * \li The bridge channel has been removed from the bridge.
+ * \li The synchronous bridge action could not be queued.
+ *
+ * \param sync_struct Synchronization object corresponding to the bridge action.
+ */
+static void bridge_sync_signal(struct bridge_sync *sync_struct)
+{
+	sem_post(&sync_struct->sem);
+}
+
 void ast_bridge_channel_lock_bridge(struct ast_bridge_channel *bridge_channel)
 {
 	struct ast_bridge *bridge;
@@ -342,6 +479,8 @@
  */
 static int bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
 {
+	ast_assert(frame->frametype != AST_FRAME_BRIDGE_ACTION_SYNC);
+
 	ast_bridge_channel_lock_bridge(bridge_channel);
 /*
  * XXX need to implement a deferred write queue for when there
@@ -493,7 +632,8 @@
  * \retval 0 on success.
  * \retval -1 on error.
  */
-static int bridge_channel_queue_action_data(struct ast_bridge_channel *bridge_channel, enum bridge_channel_action_type action, const void *data, size_t datalen)
+static int bridge_channel_queue_action_data(struct ast_bridge_channel *bridge_channel,
+	enum bridge_channel_action_type action, const void *data, size_t datalen)
 {
 	struct ast_frame frame = {
 		.frametype = AST_FRAME_BRIDGE_ACTION,
@@ -507,8 +647,10 @@
 
 /*!
  * \internal
- * \brief Write an action frame onto the bridge channel with data.
- * \since 12.0.0
+ * \brief Queue an action frame onto the bridge channel with data synchronously.
+ * \since 12.2.0
+ *
+ * The function will not return until the queued frame is freed.
  *
  * \param bridge_channel Which channel to queue the frame onto.
  * \param action Type of bridge action frame.
@@ -518,7 +660,52 @@
  * \retval 0 on success.
  * \retval -1 on error.
  */
-static int bridge_channel_write_action_data(struct ast_bridge_channel *bridge_channel, enum bridge_channel_action_type action, const void *data, size_t datalen)
+static int bridge_channel_queue_action_data_sync(struct ast_bridge_channel *bridge_channel,
+	enum bridge_channel_action_type action, const void *data, size_t datalen)
+{
+	struct sync_payload *sync_payload;
+	int sync_payload_len = sizeof(*sync_payload) + datalen;
+	struct bridge_sync sync_struct;
+	struct ast_frame frame = {
+		.frametype = AST_FRAME_BRIDGE_ACTION_SYNC,
+		.subclass.integer = action,
+	};
+
+	/* Make sure we don't end up trying to wait on ourself to deliver the frame */
+	ast_assert(!pthread_equal(pthread_self(), bridge_channel->thread));
+
+	sync_payload = ast_alloca(sync_payload_len);
+	sync_payload->id = ast_atomic_fetchadd_int(&sync_ids, +1);
+	memcpy(sync_payload->data, data, datalen);
+
+	frame.datalen = sync_payload_len;
+	frame.data.ptr = sync_payload;
+
+	bridge_sync_init(&sync_struct, sync_payload->id);
+	if (ast_bridge_channel_queue_frame(bridge_channel, &frame)) {
+		bridge_sync_cleanup(&sync_struct);
+		return -1;
+	}
+
+	bridge_sync_wait(&sync_struct);
+	bridge_sync_cleanup(&sync_struct);
+	return 0;
+}
+/*!
+ * \internal
+ * \brief Write an action frame onto the bridge channel with data.
+ * \since 12.0.0
+ *
+ * \param bridge_channel Which channel to queue the frame onto.
+ * \param action Type of bridge action frame.
+ * \param data Frame payload data to pass.
+ * \param datalen Frame payload data length to pass.
+ *
+ * \retval 0 on success.
+ * \retval -1 on error.
+ */
+static int bridge_channel_write_action_data(struct ast_bridge_channel *bridge_channel,
+	enum bridge_channel_action_type action, const void *data, size_t datalen)
 {
 	struct ast_frame frame = {
 		.frametype = AST_FRAME_BRIDGE_ACTION,
@@ -528,6 +715,27 @@
 	};
 
 	return bridge_channel_write_frame(bridge_channel, &frame);
+}
+
+static void bridge_frame_free(struct ast_frame *frame)
+{
+	if (frame->frametype == AST_FRAME_BRIDGE_ACTION_SYNC) {
+		struct sync_payload *sync_payload = frame->data.ptr;
+		struct bridge_sync *sync;
+
+		AST_RWLIST_RDLOCK(&sync_structs);
+		AST_RWLIST_TRAVERSE(&sync_structs, sync, list) {
+			if (sync->id == sync_payload->id) {
+				break;
+			}
+		}
+		if (sync) {
+			bridge_sync_signal(sync);
+		}
+		AST_RWLIST_UNLOCK(&sync_structs);
+	}
+
+	ast_frfree(frame);
 }
 
 int ast_bridge_channel_queue_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *fr)
@@ -557,7 +765,7 @@
 	if (bridge_channel->state != BRIDGE_CHANNEL_STATE_WAIT) {
 		/* Drop frames on channels leaving the bridge. */
 		ast_bridge_channel_unlock(bridge_channel);
-		ast_frfree(dup);
+		bridge_frame_free(dup);
 		return 0;
 	}
 
@@ -816,7 +1024,7 @@
 	size_t len_payload = sizeof(*payload) + len_name + len_moh;
 
 	/* Fill in play file frame data. */
-	payload = alloca(len_payload);
+	payload = ast_alloca(len_payload);
 	payload->custom_play = custom_play;
 	payload->moh_offset = len_moh ? len_name : 0;
 	strcpy(payload->playfile, playfile);/* Safe */
@@ -836,6 +1044,13 @@
 int ast_bridge_channel_queue_playfile(struct ast_bridge_channel *bridge_channel, ast_bridge_custom_play_fn custom_play, const char *playfile, const char *moh_class)
 {
 	return payload_helper_playfile(bridge_channel_queue_action_data,
+		bridge_channel, custom_play, playfile, moh_class);
+}
+
+int ast_bridge_channel_queue_playfile_sync(struct ast_bridge_channel *bridge_channel,
+		ast_bridge_custom_play_fn custom_play, const char *playfile, const char *moh_class)
+{
+	return payload_helper_playfile(bridge_channel_queue_action_data_sync,
 		bridge_channel, custom_play, playfile, moh_class);
 }
 
@@ -1389,53 +1604,55 @@
  *
  * \param bridge_channel Channel to execute the action on.
  * \param action What to do.
+ * \param data data from the action.
  *
  * \return Nothing
  */
-static void bridge_channel_handle_action(struct ast_bridge_channel *bridge_channel, struct ast_frame *action)
-{
-	switch (action->subclass.integer) {
+static void bridge_channel_handle_action(struct ast_bridge_channel *bridge_channel,
+	enum bridge_channel_action_type action, void *data)
+{
+	switch (action) {
 	case BRIDGE_CHANNEL_ACTION_DTMF_STREAM:
 		bridge_channel_suspend(bridge_channel);
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
-		bridge_channel_dtmf_stream(bridge_channel, action->data.ptr);
+		bridge_channel_dtmf_stream(bridge_channel, data);
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
 		bridge_channel_unsuspend(bridge_channel);
 		break;
 	case BRIDGE_CHANNEL_ACTION_TALKING_START:
 	case BRIDGE_CHANNEL_ACTION_TALKING_STOP:
 		bridge_channel_talking(bridge_channel,
-			action->subclass.integer == BRIDGE_CHANNEL_ACTION_TALKING_START);
+			action == BRIDGE_CHANNEL_ACTION_TALKING_START);
 		break;
 	case BRIDGE_CHANNEL_ACTION_PLAY_FILE:
 		bridge_channel_suspend(bridge_channel);
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
-		bridge_channel_playfile(bridge_channel, action->data.ptr);
+		bridge_channel_playfile(bridge_channel, data);
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
 		bridge_channel_unsuspend(bridge_channel);
 		break;
 	case BRIDGE_CHANNEL_ACTION_RUN_APP:
 		bridge_channel_suspend(bridge_channel);
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
-		bridge_channel_run_app(bridge_channel, action->data.ptr);
+		bridge_channel_run_app(bridge_channel, data);
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
 		bridge_channel_unsuspend(bridge_channel);
 		break;
 	case BRIDGE_CHANNEL_ACTION_CALLBACK:
-		bridge_channel_do_callback(bridge_channel, action->data.ptr);
+		bridge_channel_do_callback(bridge_channel, data);
 		break;
 	case BRIDGE_CHANNEL_ACTION_PARK:
 		bridge_channel_suspend(bridge_channel);
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
-		bridge_channel_park(bridge_channel, action->data.ptr);
+		bridge_channel_park(bridge_channel, data);
 		ast_indicate(bridge_channel->chan, AST_CONTROL_SRCUPDATE);
 		bridge_channel_unsuspend(bridge_channel);
 		break;
 	case BRIDGE_CHANNEL_ACTION_BLIND_TRANSFER:
-		bridge_channel_blind_transfer(bridge_channel, action->data.ptr);
+		bridge_channel_blind_transfer(bridge_channel, data);
 		break;
 	case BRIDGE_CHANNEL_ACTION_ATTENDED_TRANSFER:
-		bridge_channel_attended_transfer(bridge_channel, action->data.ptr);
+		bridge_channel_attended_transfer(bridge_channel, data);
 		break;
 	default:
 		break;
@@ -1700,6 +1917,7 @@
 {
 	struct ast_frame *fr;
 	char nudge;
+	struct sync_payload *sync_payload;
 
 	ast_bridge_channel_lock(bridge_channel);
 	if (read(bridge_channel->alert_pipe[0], &nudge, sizeof(nudge)) < 0) {
@@ -1715,7 +1933,11 @@
 	}
 	switch (fr->frametype) {
 	case AST_FRAME_BRIDGE_ACTION:
-		bridge_channel_handle_action(bridge_channel, fr);
+		bridge_channel_handle_action(bridge_channel, fr->subclass.integer, fr->data.ptr);
+		break;
+	case AST_FRAME_BRIDGE_ACTION_SYNC:
+		sync_payload = fr->data.ptr;
+		bridge_channel_handle_action(bridge_channel, fr->subclass.integer, sync_payload->data);
 		break;
 	case AST_FRAME_CONTROL:
 		bridge_channel_handle_control(bridge_channel, fr);
@@ -1728,7 +1950,7 @@
 		ast_write(bridge_channel->chan, fr);
 		break;
 	}
-	ast_frfree(fr);
+	bridge_frame_free(fr);
 }
 
 /*! \brief Internal function to handle DTMF from a channel */
@@ -1745,7 +1967,7 @@
 	if (hook) {
 		enum ast_frame_type frametype = frame->frametype;
 
-		ast_frfree(frame);
+		bridge_frame_free(frame);
 		frame = NULL;
 
 		ao2_ref(hook, -1);
@@ -1805,7 +2027,7 @@
 		switch (frame->subclass.integer) {
 		case AST_CONTROL_HANGUP:
 			ast_bridge_channel_kick(bridge_channel, 0);
-			ast_frfree(frame);
+			bridge_frame_free(frame);
 			return;
 		default:
 			break;
@@ -1818,7 +2040,7 @@
 			return;
 		}
 		if (!bridge_channel->features->dtmf_passthrough) {
-			ast_frfree(frame);
+			bridge_frame_free(frame);
 			return;
 		}
 		break;
@@ -1828,7 +2050,7 @@
 
 	/* Simply write the frame out to the bridge technology. */
 	bridge_channel_write_frame(bridge_channel, frame);
-	ast_frfree(frame);
+	bridge_frame_free(frame);
 }
 
 /*!
@@ -2205,7 +2427,7 @@
 
 	/* Flush any unhandled wr_queue frames. */
 	while ((fr = AST_LIST_REMOVE_HEAD(&bridge_channel->wr_queue, frame_list))) {
-		ast_frfree(fr);
+		bridge_frame_free(fr);
 	}
 	pipe_close(bridge_channel->alert_pipe);
 

Modified: branches/12/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/main/channel.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/main/channel.c (original)
+++ branches/12/main/channel.c Mon Mar 17 11:52:12 2014
@@ -1531,6 +1531,7 @@
 	 */
 	switch (frame->frametype) {
 	case AST_FRAME_BRIDGE_ACTION:
+	case AST_FRAME_BRIDGE_ACTION_SYNC:
 	case AST_FRAME_CONTROL:
 	case AST_FRAME_TEXT:
 	case AST_FRAME_IMAGE:
@@ -2875,6 +2876,7 @@
 				case AST_FRAME_CONTROL:
 				case AST_FRAME_IAX:
 				case AST_FRAME_BRIDGE_ACTION:
+				case AST_FRAME_BRIDGE_ACTION_SYNC:
 				case AST_FRAME_NULL:
 				case AST_FRAME_CNG:
 					break;

Modified: branches/12/main/frame.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/main/frame.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/main/frame.c (original)
+++ branches/12/main/frame.c Mon Mar 17 11:52:12 2014
@@ -639,6 +639,10 @@
 		/* Should never happen */
 		snprintf(subclass, slen, "Bridge Frametype %d", f->subclass.integer);
 		break;
+	case AST_FRAME_BRIDGE_ACTION_SYNC:
+		/* Should never happen */
+		snprintf(subclass, slen, "Synchronous Bridge Frametype %d", f->subclass.integer);
+		break;
 	case AST_FRAME_TEXT:
 		ast_copy_string(subclass, "N/A", slen);
 		if (moreinfo) {
@@ -727,6 +731,10 @@
 		ast_copy_string(ftype, "IAX Specific", len);
 		break;
 	case AST_FRAME_BRIDGE_ACTION:
+		/* Should never happen */
+		ast_copy_string(ftype, "Bridge Specific", len);
+		break;
+	case AST_FRAME_BRIDGE_ACTION_SYNC:
 		/* Should never happen */
 		ast_copy_string(ftype, "Bridge Specific", len);
 		break;

Modified: branches/12/main/sorcery.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/main/sorcery.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/main/sorcery.c (original)
+++ branches/12/main/sorcery.c Mon Mar 17 11:52:12 2014
@@ -580,6 +580,14 @@
 	}
 
 	strcpy(sorcery->module_name, module_name); /* Safe */
+
+	if (__ast_sorcery_apply_config(sorcery, module_name, module_name) == AST_SORCERY_APPLY_FAIL) {
+		ast_log(LOG_ERROR, "Error attempting to apply configuration %s to sorcery.", module_name);
+		ao2_cleanup(sorcery);
+		sorcery = NULL;
+		goto done;
+	}
+
 	ao2_link_flags(instances, sorcery, OBJ_NOLOCK);
 
 done:
@@ -623,7 +631,7 @@
 	}
 
 	/* Order matters for object wizards */
-	if (!(object_type->wizards = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, 1, NULL, NULL))) {
+	if (!(object_type->wizards = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_NOLOCK, 1, NULL, sorcery_wizard_cmp))) {
 		ao2_ref(object_type, -1);
 		return NULL;
 	}
@@ -683,7 +691,8 @@
 }
 
 /*! \brief Internal function which creates an object type and adds a wizard mapping */
-static int sorcery_apply_wizard_mapping(struct ast_sorcery *sorcery, const char *type, const char *module, const char *name, const char *data, unsigned int caching)
+static enum ast_sorcery_apply_result sorcery_apply_wizard_mapping(struct ast_sorcery *sorcery,
+		const char *type, const char *module, const char *name, const char *data, unsigned int caching)
 {
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
 	RAII_VAR(struct ast_sorcery_wizard *, wizard, ao2_find(wizards, name, OBJ_KEY), ao2_cleanup);
@@ -691,18 +700,30 @@
 	int created = 0;
 
 	if (!wizard || !object_wizard) {
-		return -1;
+		return AST_SORCERY_APPLY_FAIL;
 	}
 
 	if (!object_type) {
 		if (!(object_type = sorcery_object_type_alloc(type, module))) {
-			return -1;
+			return AST_SORCERY_APPLY_FAIL;
 		}
 		created = 1;
 	}
 
+	if (!created) {
+		struct ast_sorcery_wizard *found;
+
+		found = ao2_find(object_type->wizards, wizard, OBJ_SEARCH_OBJECT);
+		if (found) {
+			ast_debug(1, "Wizard %s already applied to object type %s\n",
+					wizard->name, object_type->name);
+			ao2_cleanup(found);
+			return AST_SORCERY_APPLY_DUPLICATE;
+		}
+	}
+
 	if (wizard->open && !(object_wizard->data = wizard->open(data))) {
-		return -1;
+		return AST_SORCERY_APPLY_FAIL;
 	}
 
 	ast_module_ref(wizard->module);
@@ -716,18 +737,18 @@
 		ao2_link(sorcery->types, object_type);
 	}
 
-	return 0;
-}
-
-int __ast_sorcery_apply_config(struct ast_sorcery *sorcery, const char *name, const char *module)
+	return AST_SORCERY_APPLY_SUCCESS;
+}
+
+enum ast_sorcery_apply_result  __ast_sorcery_apply_config(struct ast_sorcery *sorcery, const char *name, const char *module)
 {
 	struct ast_flags flags = { 0 };
 	struct ast_config *config = ast_config_load2("sorcery.conf", "sorcery", flags);
 	struct ast_variable *mapping;
-	int res = 0;
+	int res = AST_SORCERY_APPLY_SUCCESS;
 
 	if (!config || config == CONFIG_STATUS_FILEINVALID) {
-		return -1;
+		return AST_SORCERY_APPLY_FAIL;
 	}
 
 	for (mapping = ast_variable_browse(config, name); mapping; mapping = mapping->next) {
@@ -750,8 +771,8 @@
 		}
 
 		/* Any error immediately causes us to stop */
-		if (sorcery_apply_wizard_mapping(sorcery, type, module, wizard, data, caching)) {
-			res = -1;
+		if (sorcery_apply_wizard_mapping(sorcery, type, module, wizard, data, caching) == AST_SORCERY_APPLY_FAIL) {
+			res = AST_SORCERY_APPLY_FAIL;
 			break;
 		}
 	}
@@ -761,13 +782,13 @@
 	return res;
 }
 
-int __ast_sorcery_apply_default(struct ast_sorcery *sorcery, const char *type, const char *module, const char *name, const char *data)
+enum ast_sorcery_apply_result __ast_sorcery_apply_default(struct ast_sorcery *sorcery, const char *type, const char *module, const char *name, const char *data)
 {
 	RAII_VAR(struct ast_sorcery_object_type *, object_type, ao2_find(sorcery->types, type, OBJ_KEY), ao2_cleanup);
 
 	/* Defaults can not be added if any existing mapping exists */
 	if (object_type) {
-		return -1;
+		return AST_SORCERY_APPLY_DEFAULT_UNNECESSARY;
 	}
 
 	return sorcery_apply_wizard_mapping(sorcery, type, module, name, data, 0);

Modified: branches/12/res/res_mwi_external.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_mwi_external.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/res/res_mwi_external.c (original)
+++ branches/12/res/res_mwi_external.c Mon Mar 17 11:52:12 2014
@@ -163,10 +163,8 @@
 	}
 
 	/* Map the external MWI wizards. */
-	res = !!ast_sorcery_apply_config(mwi_sorcery, "res_mwi_external");
-	res &= !!ast_sorcery_apply_default(mwi_sorcery, MWI_MAILBOX_TYPE, "astdb",
-		MWI_ASTDB_PREFIX);
-	if (res) {
+	if (ast_sorcery_apply_default(mwi_sorcery, MWI_MAILBOX_TYPE, "astdb",
+			MWI_ASTDB_PREFIX) == AST_SORCERY_APPLY_FAIL) {
 		ast_log(LOG_ERROR, "MWI external: Sorcery could not setup wizards.\n");
 		return -1;
 	}

Modified: branches/12/res/res_pjsip/config_system.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_pjsip/config_system.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/res/res_pjsip/config_system.c (original)
+++ branches/12/res/res_pjsip/config_system.c Mon Mar 17 11:52:12 2014
@@ -116,8 +116,6 @@
 		return -1;
 	}
 
-	ast_sorcery_apply_config(system_sorcery, "res_pjsip");
-
 	ast_sorcery_apply_default(system_sorcery, "system", "config", "pjsip.conf,criteria=type=system");
 
 	if (ast_sorcery_object_register_no_reload(system_sorcery, "system", system_alloc, NULL, system_apply)) {

Modified: branches/12/res/res_pjsip/pjsip_configuration.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_pjsip/pjsip_configuration.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/res/res_pjsip/pjsip_configuration.c (original)
+++ branches/12/res/res_pjsip/pjsip_configuration.c Mon Mar 17 11:52:12 2014
@@ -1623,8 +1623,6 @@
 		ast_log(LOG_ERROR, "Failed to open SIP sorcery failed to open\n");
 		return -1;
 	}
-
-	ast_sorcery_apply_config(sip_sorcery, "res_pjsip");
 
 	ast_sip_initialize_cli();
 

Modified: branches/12/res/res_stasis_playback.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_stasis_playback.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/res/res_stasis_playback.c (original)
+++ branches/12/res/res_stasis_playback.c Mon Mar 17 11:52:12 2014
@@ -78,14 +78,10 @@
 	long offsetms;
 	/*! Number of milliseconds to skip for forward/reverse operations */
 	int skipms;
-	/*! Condition for waiting on done to be set */
-	ast_cond_t done_cond;
 	/*! Number of milliseconds of media that has been played */
 	long playedms;
 	/*! Current playback state */
 	enum stasis_app_playback_state state;
-	/*! Set when playback has been completed */
-	unsigned int done:1;
 	/*! Set when the playback can be controlled */
 	unsigned int controllable:1;
 };
@@ -121,7 +117,6 @@
 	struct stasis_app_playback *playback = obj;
 
 	ast_string_field_free_memory(playback);
-	ast_cond_destroy(&playback->done_cond);
 }
 
 static struct stasis_app_playback *playback_create(
@@ -129,7 +124,6 @@
 {
 	RAII_VAR(struct stasis_app_playback *, playback, NULL, ao2_cleanup);
 	char uuid[AST_UUID_STR_LEN];
-	int res;
 
 	if (!control) {
 		return NULL;
@@ -137,13 +131,6 @@
 
 	playback = ao2_alloc(sizeof(*playback), playback_dtor);
 	if (!playback || ast_string_field_init(playback, 128)) {
-		return NULL;
-	}
-
-	res = ast_cond_init(&playback->done_cond, NULL);
-	if (res != 0) {
-		ast_log(LOG_ERROR, "Error creating done condition: %s\n",
-			strerror(errno));
 		return NULL;
 	}
 
@@ -266,21 +253,9 @@
 	playback_publish(playback);
 }
 
-/*!
- * \brief RAII_VAR function to mark a playback as done when leaving scope.
- */
-static void mark_as_done(struct stasis_app_playback *playback)
-{
-	SCOPED_AO2LOCK(lock, playback);
-	playback->done = 1;
-	ast_cond_broadcast(&playback->done_cond);
-}
-
 static void play_on_channel(struct stasis_app_playback *playback,
 	struct ast_channel *chan)
 {
-	RAII_VAR(struct stasis_app_playback *, mark_when_done, playback,
-		mark_as_done);
 	int res;
 	long offsetms;
 
@@ -399,7 +374,6 @@
 	RAII_VAR(struct stasis_app_playback *, playback, NULL,
 		remove_from_playbacks);
 	struct ast_bridge *bridge;
-	int res;
 
 	playback = data;
 
@@ -413,28 +387,16 @@
 
 		/* Queue up playback on the bridge */
 		ast_bridge_lock(bridge);
-		bridge_chan = bridge_find_channel(bridge, chan);
+		bridge_chan = ao2_bump(bridge_find_channel(bridge, chan));
+		ast_bridge_unlock(bridge);
 		if (bridge_chan) {
-			ast_bridge_channel_queue_playfile(
+			ast_bridge_channel_queue_playfile_sync(
 				bridge_chan,
 				play_on_channel_in_bridge,
 				playback->id,
 				NULL); /* moh_class */
 		}
-		ast_bridge_unlock(bridge);
-
-		/* Wait for playback to complete */
-		ao2_lock(playback);
-		while (!playback->done) {
-			res = ast_cond_wait(&playback->done_cond,
-				ao2_object_get_lockaddr(playback));
-			if (res != 0) {
-				ast_log(LOG_ERROR,
-					"Error waiting for playback to complete: %s\n",
-					strerror(errno));
-			}
-		}
-		ao2_unlock(playback);
+		ao2_cleanup(bridge_chan);
 	} else {
 		play_on_channel(playback, chan);
 	}

Modified: branches/12/tests/test_sorcery.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/tests/test_sorcery.c?view=diff&rev=410673&r1=410672&r2=410673
==============================================================================
--- branches/12/tests/test_sorcery.c (original)
+++ branches/12/tests/test_sorcery.c Mon Mar 17 11:52:12 2014
@@ -306,7 +306,7 @@
 		return NULL;
 	}
 
-	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL) ||
+	if ((ast_sorcery_apply_default(sorcery, "test", "memory", NULL) != AST_SORCERY_APPLY_SUCCESS) ||
 		ast_sorcery_internal_object_register(sorcery, "test", test_sorcery_object_alloc, NULL, NULL)) {
 		ast_sorcery_unref(sorcery);
 		return NULL;
@@ -452,17 +452,17 @@
 		return AST_TEST_FAIL;
 	}
 
-	if (!ast_sorcery_apply_default(sorcery, "test", "dummy", NULL)) {
+	if (ast_sorcery_apply_default(sorcery, "test", "dummy", NULL) != AST_SORCERY_APPLY_FAIL) {
 		ast_test_status_update(test, "Successfully set a default wizard that doesn't exist\n");
 		return AST_TEST_FAIL;
 	}
 
-	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL)) {
+	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL) != AST_SORCERY_APPLY_SUCCESS) {
 		ast_test_status_update(test, "Failed to set a known wizard as a default\n");
 		return AST_TEST_FAIL;
 	}
 
-	if (!ast_sorcery_apply_default(sorcery, "test", "memory", NULL)) {
+	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL) != AST_SORCERY_APPLY_DEFAULT_UNNECESSARY) {
 		ast_test_status_update(test, "Successfully set a default wizard on a type twice\n");
 		return AST_TEST_FAIL;
 	}
@@ -493,7 +493,7 @@
 		return AST_TEST_NOT_RUN;
 	}
 
-	if (!ast_category_get(config, "test_sorcery")) {
+	if (!ast_category_get(config, "test_sorcery_section")) {
 		ast_test_status_update(test, "Sorcery configuration file does not have test_sorcery section\n");
 		ast_config_destroy(config);
 		return AST_TEST_NOT_RUN;
@@ -506,7 +506,7 @@
 		return AST_TEST_FAIL;
 	}
 
-	if (ast_sorcery_apply_config(sorcery, "test_sorcery")) {
+	if (ast_sorcery_apply_config(sorcery, "test_sorcery_section") != AST_SORCERY_APPLY_SUCCESS) {
 		ast_test_status_update(test, "Failed to apply configured object mappings\n");
 		return AST_TEST_FAIL;
 	}
@@ -535,7 +535,7 @@
 		return AST_TEST_FAIL;
 	}
 
-	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL)) {
+	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL) != AST_SORCERY_APPLY_SUCCESS) {
 		ast_test_status_update(test, "Failed to set a known wizard as a default\n");
 		return AST_TEST_FAIL;
 	}
@@ -608,7 +608,7 @@
 		return AST_TEST_FAIL;
 	}
 
-	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL)) {
+	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL) != AST_SORCERY_APPLY_SUCCESS) {
 		ast_test_status_update(test, "Failed to set a known wizard as a default\n");
 		return AST_TEST_FAIL;
 	}
@@ -657,7 +657,7 @@
 		return AST_TEST_FAIL;
 	}
 
-	if (ast_sorcery_apply_default(sorcery, "test", "memory", NULL)) {

[... 134 lines stripped ...]



More information about the asterisk-commits mailing list