[asterisk-commits] kmoore: branch 12 r412193 - in /branches/12: apps/ include/asterisk/ main/ re...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Apr 11 07:36:01 CDT 2014
Author: kmoore
Date: Fri Apr 11 07:35:52 2014
New Revision: 412193
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=412193
Log:
bridging: Ensure locking during snapshot creation
While the vast majority of bridge snapshot creation is locked properly,
there are currently some instances that are not. This adds the missing
locking to ensure bridge state is not malleable during snapshot
creation.
(closes issue ASTERISK-22904)
Review: https://reviewboard.asterisk.org/r/3415/
Reported by: Matt Jordan
Modified:
branches/12/apps/app_confbridge.c
branches/12/include/asterisk/stasis_bridges.h
branches/12/main/bridge.c
branches/12/main/bridge_basic.c
branches/12/res/ari/resource_bridges.c
branches/12/tests/test_cel.c
Modified: branches/12/apps/app_confbridge.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/apps/app_confbridge.c?view=diff&rev=412193&r1=412192&r2=412193
==============================================================================
--- branches/12/apps/app_confbridge.c (original)
+++ branches/12/apps/app_confbridge.c Fri Apr 11 07:35:52 2014
@@ -439,10 +439,13 @@
ast_json_object_update(json_object, extras);
}
+ ast_bridge_lock(conference->bridge);
msg = ast_bridge_blob_create(type,
conference->bridge,
chan,
json_object);
+ ast_bridge_unlock(conference->bridge);
+
if (!msg) {
return;
}
Modified: branches/12/include/asterisk/stasis_bridges.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/include/asterisk/stasis_bridges.h?view=diff&rev=412193&r1=412192&r2=412193
==============================================================================
--- branches/12/include/asterisk/stasis_bridges.h (original)
+++ branches/12/include/asterisk/stasis_bridges.h Fri Apr 11 07:35:52 2014
@@ -65,6 +65,8 @@
* \brief Generate a snapshot of the bridge state. This is an ao2 object, so
* ao2_cleanup() to deallocate.
*
+ * \pre Bridge is locked
+ *
* \param bridge The bridge from which to generate a snapshot
*
* \retval AO2 refcounted snapshot on success
@@ -135,6 +137,8 @@
/*!
* \since 12
* \brief Publish the state of a bridge
+ *
+ * \pre Bridge is locked
*
* \param bridge The bridge for which to publish state
*/
@@ -157,6 +161,8 @@
/*!
* \since 12
* \brief Publish a bridge merge
+ *
+ * \pre Bridges involved are locked
*
* \param to The bridge to which channels are being added
* \param from The bridge from which channels are being removed
@@ -281,7 +287,7 @@
/*!
* \brief Publish a blind transfer event
*
- * \pre No channels or bridges are locked
+ * \pre Bridges involved are locked. Channels involved are not locked.
*
* \param is_external Whether the blind transfer was initiated externally (e.g. via AMI or native protocol)
* \param result The success or failure of the transfer
@@ -346,7 +352,7 @@
* Publish an \ref ast_attended_transfer_message with the dest_type set to
* \c AST_ATTENDED_TRANSFER_DEST_FAIL.
*
- * \pre No channels or bridges are locked
+ * \pre Bridges involved are locked. Channels involved are not locked.
*
* \param is_external Indicates if the transfer was initiated externally
* \param result The result of the transfer. Will always be a type of failure.
@@ -369,7 +375,7 @@
*
* In either case, two bridges enter, one leaves.
*
- * \pre No channels or bridges are locked
+ * \pre Bridges involved are locked. Channels involved are not locked.
*
* \param is_external Indicates if the transfer was initiated externally
* \param result The result of the transfer.
@@ -390,7 +396,7 @@
* this results from merging two bridges together. The difference is that a
* transferer channel survives the bridge merge
*
- * \pre No channels or bridges are locked
+ * \pre Bridges involved are locked. Channels involved are not locked.
*
* \param is_external Indicates if the transfer was initiated externally
* \param result The result of the transfer.
@@ -413,7 +419,7 @@
* \li A transferee channel leaving a bridge to run an app
* \li A bridge of transferees running an app (via a local channel)
*
- * \pre No channels or bridges are locked
+ * \pre Bridges involved are locked. Channels involved are not locked.
*
* \param is_external Indicates if the transfer was initiated externally
* \param result The result of the transfer.
@@ -438,7 +444,7 @@
* When this type of transfer occurs, the two bridges continue to exist after the
* transfer and a local channel is used to link the two bridges together.
*
- * \pre No channels or bridges are locked
+ * \pre Bridges involved are locked. Channels involved are not locked.
*
* \param is_external Indicates if the transfer was initiated externally
* \param result The result of the transfer.
Modified: branches/12/main/bridge.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/main/bridge.c?view=diff&rev=412193&r1=412192&r2=412193
==============================================================================
--- branches/12/main/bridge.c (original)
+++ branches/12/main/bridge.c Fri Apr 11 07:35:52 2014
@@ -615,7 +615,10 @@
{
RAII_VAR(struct ast_bridge_snapshot *, snapshot, NULL, ao2_cleanup);
+ ast_bridge_lock(bridge);
snapshot = ast_bridge_snapshot_create(bridge);
+ ast_bridge_unlock(bridge);
+
if (!snapshot) {
return NULL;
}
@@ -689,7 +692,9 @@
{
if (bridge) {
bridge->construction_completed = 1;
+ ast_bridge_lock(bridge);
ast_bridge_publish_state(bridge);
+ ast_bridge_unlock(bridge);
if (!ao2_link(bridges, bridge)) {
ast_bridge_destroy(bridge, 0);
bridge = NULL;
@@ -4067,7 +4072,9 @@
struct ast_bridge_channel_pair pair;
pair.channel = transferer;
pair.bridge = bridge;
+ ast_bridge_lock(bridge);
ast_bridge_publish_blind_transfer(is_external, result, &pair, context, exten);
+ ast_bridge_unlock(bridge);
}
enum ast_transfer_result ast_bridge_transfer_blind(int is_external,
@@ -4294,7 +4301,7 @@
RAII_VAR(struct ast_bridge_channel *, to_target_bridge_channel, NULL, ao2_cleanup);
RAII_VAR(struct ao2_container *, channels, NULL, ao2_cleanup);
RAII_VAR(struct ast_channel *, transferee, NULL, ao2_cleanup);
- struct ast_bridge *the_bridge;
+ struct ast_bridge *the_bridge = NULL;
struct ast_channel *chan_bridged;
struct ast_channel *chan_unbridged;
int transfer_prohibited;
@@ -4412,8 +4419,10 @@
set_transfer_variables_all(to_transferee, channels, 1);
if (do_bridge_transfer) {
- res = attended_transfer_bridge(chan_bridged, chan_unbridged, the_bridge, NULL, &publication);
- goto end;
+ ast_bridge_lock(the_bridge);
+ res = attended_transfer_bridge(chan_bridged, chan_unbridged, the_bridge, NULL, &publication);
+ ast_bridge_unlock(the_bridge);
+ goto end;
}
transferee = get_transferee(channels, chan_bridged);
@@ -4430,7 +4439,9 @@
ast_bridge_remove(the_bridge, chan_bridged);
+ ast_bridge_lock(the_bridge);
publish_attended_transfer_app(&publication, app);
+ ast_bridge_unlock(the_bridge);
res = AST_BRIDGE_TRANSFER_SUCCESS;
end:
@@ -4438,7 +4449,20 @@
* All failure paths have deferred publishing a stasis message until this point
*/
if (res != AST_BRIDGE_TRANSFER_SUCCESS) {
+ if (to_transferee_bridge && to_target_bridge) {
+ ast_bridge_lock_both(to_transferee_bridge, to_target_bridge);
+ } else if (the_bridge) {
+ ast_bridge_lock(the_bridge);
+ }
+
publish_attended_transfer_fail(&publication, res);
+
+ if (to_transferee_bridge && to_target_bridge) {
+ ast_bridge_unlock(to_transferee_bridge);
+ ast_bridge_unlock(to_target_bridge);
+ } else if (the_bridge) {
+ ast_bridge_unlock(the_bridge);
+ }
}
stasis_publish_data_cleanup(&publication);
return res;
Modified: branches/12/main/bridge_basic.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/main/bridge_basic.c?view=diff&rev=412193&r1=412192&r2=412193
==============================================================================
--- branches/12/main/bridge_basic.c (original)
+++ branches/12/main/bridge_basic.c Fri Apr 11 07:35:52 2014
@@ -1534,8 +1534,11 @@
.bridge = props->target_bridge,
};
+ ast_bridge_lock_both(transferee.bridge, transfer_target.bridge);
ast_bridge_publish_attended_transfer_bridge_merge(0, AST_BRIDGE_TRANSFER_SUCCESS,
&transferee, &transfer_target, props->transferee_bridge);
+ ast_bridge_unlock(transferee.bridge);
+ ast_bridge_unlock(transfer_target.bridge);
}
/*!
@@ -1556,8 +1559,11 @@
.bridge = props->transferee_bridge,
};
+ ast_bridge_lock_both(transferee.bridge, transfer_target.bridge);
ast_bridge_publish_attended_transfer_threeway(0, AST_BRIDGE_TRANSFER_SUCCESS,
&transferee, &transfer_target, &threeway);
+ ast_bridge_unlock(transferee.bridge);
+ ast_bridge_unlock(transfer_target.bridge);
}
/*!
@@ -1574,8 +1580,11 @@
.bridge = props->target_bridge,
};
+ ast_bridge_lock_both(transferee.bridge, transfer_target.bridge);
ast_bridge_publish_attended_transfer_fail(0, AST_BRIDGE_TRANSFER_FAIL,
&transferee, &transfer_target);
+ ast_bridge_unlock(transferee.bridge);
+ ast_bridge_unlock(transfer_target.bridge);
}
/*!
Modified: branches/12/res/ari/resource_bridges.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/ari/resource_bridges.c?view=diff&rev=412193&r1=412192&r2=412193
==============================================================================
--- branches/12/res/ari/resource_bridges.c (original)
+++ branches/12/res/ari/resource_bridges.c Fri Apr 11 07:35:52 2014
@@ -746,7 +746,10 @@
return;
}
+ ast_bridge_lock(bridge);
snapshot = ast_bridge_snapshot_create(bridge);
+ ast_bridge_unlock(bridge);
+
if (!snapshot) {
ast_ari_response_error(
response, 500, "Internal Error",
@@ -792,7 +795,10 @@
return;
}
+ ast_bridge_lock(bridge);
snapshot = ast_bridge_snapshot_create(bridge);
+ ast_bridge_unlock(bridge);
+
if (!snapshot) {
ast_ari_response_error(
response, 500, "Internal Error",
Modified: branches/12/tests/test_cel.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/tests/test_cel.c?view=diff&rev=412193&r1=412192&r2=412193
==============================================================================
--- branches/12/tests/test_cel.c (original)
+++ branches/12/tests/test_cel.c Fri Apr 11 07:35:52 2014
@@ -1213,8 +1213,10 @@
pair.bridge = bridge;
pair.channel = chan_alice;
+ ast_bridge_lock(bridge);
ast_bridge_publish_blind_transfer(1, AST_BRIDGE_TRANSFER_SUCCESS,
&pair, "transfer_context", "transfer_extension");
+ ast_bridge_unlock(bridge);
BLINDTRANSFER_EVENT(chan_alice, bridge, "transfer_extension", "transfer_context");
BRIDGE_EXIT(chan_alice, bridge);
More information about the asterisk-commits
mailing list