From c88b7945f64ef713da3791e2518608c1e65e90de Mon Sep 17 00:00:00 2001 From: Richard Mudgett <rmudgett@digium.com> Date: Sat, 8 Jun 2013 05:18:22 +0000 Subject: [PATCH] Fix a crash when a bridge switches from the softmix bridge technology to another. A three party bridge uses the softmix bridging technology. This technology has a dedicated thread used to perform the analog mixing. When one of these parties leaves the bridge, the bridge technology is changed from the softmix technology to a two-party mixing technology. Changing technologies is done by removing channels from the old technology and adding them to the new technology. Since the remaining channels do not leave the bridge, the softmix mixing thread could continue to process all channels in the bridge. If the bridge code is not able to start destruction of the softmix technology before the softmix mixing thread wakes up, a crash happens. * Added a stop technology callback that technologies can use to request any helper threads to stop in preparation for being destroyed. (closes issue AST-1156) Reported by: John Bigelow git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@390975 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- bridges/bridge_softmix.c | 24 ++++++++++++++++++++++++ include/asterisk/bridging_technology.h | 6 ++++++ main/bridging.c | 17 +++++++++++++++-- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c index 2a65e0f86f..ac183b28a0 100644 --- a/bridges/bridge_softmix.c +++ b/bridges/bridge_softmix.c @@ -1064,6 +1064,29 @@ static int softmix_bridge_create(struct ast_bridge *bridge) return 0; } +/*! + * \internal + * \brief Request the softmix mixing thread stop. + * \since 12.0.0 + * + * \param bridge Which bridge is being stopped. + * + * \return Nothing + */ +static void softmix_bridge_stop(struct ast_bridge *bridge) +{ + struct softmix_bridge_data *softmix_data; + + softmix_data = bridge->tech_pvt; + if (!softmix_data) { + return; + } + + ast_mutex_lock(&softmix_data->lock); + softmix_data->stop = 1; + ast_mutex_unlock(&softmix_data->lock); +} + /*! \brief Function called when a bridge is destroyed */ static void softmix_bridge_destroy(struct ast_bridge *bridge) { @@ -1096,6 +1119,7 @@ static struct ast_bridge_technology softmix_bridge = { .capabilities = AST_BRIDGE_CAPABILITY_MULTIMIX, .preference = AST_BRIDGE_PREFERENCE_BASE_MULTIMIX, .create = softmix_bridge_create, + .stop = softmix_bridge_stop, .destroy = softmix_bridge_destroy, .join = softmix_bridge_join, .leave = softmix_bridge_leave, diff --git a/include/asterisk/bridging_technology.h b/include/asterisk/bridging_technology.h index 8b9bbe0d02..6405545dd9 100644 --- a/include/asterisk/bridging_technology.h +++ b/include/asterisk/bridging_technology.h @@ -62,6 +62,12 @@ struct ast_bridge_technology { * However, it can be accessed as if it were locked. */ int (*create)(struct ast_bridge *bridge); + /*! + * \brief Request a bridge technology instance stop in preparation for being destroyed. + * + * \note On entry, bridge is already locked. + */ + void (*stop)(struct ast_bridge *bridge); /*! * \brief Destroy a bridging technology instance for a bridge. * diff --git a/main/bridging.c b/main/bridging.c index 6a21c0b64f..51f52c00ea 100644 --- a/main/bridging.c +++ b/main/bridging.c @@ -1325,6 +1325,13 @@ static void destroy_bridge(void *obj) /* Pass off the bridge to the technology to destroy if needed */ if (bridge->technology) { + ast_debug(1, "Bridge %s: calling %s technology stop\n", + bridge->uniqueid, bridge->technology->name); + if (bridge->technology->stop) { + ast_bridge_lock(bridge); + bridge->technology->stop(bridge); + ast_bridge_unlock(bridge); + } ast_debug(1, "Bridge %s: calling %s technology destructor\n", bridge->uniqueid, bridge->technology->name); if (bridge->technology->destroy) { @@ -1743,6 +1750,12 @@ static int smart_bridge_operation(struct ast_bridge *bridge) return -1; } + ast_debug(1, "Bridge %s: calling %s technology stop\n", + dummy_bridge.uniqueid, old_technology->name); + if (old_technology->stop) { + old_technology->stop(&dummy_bridge); + } + /* Move existing channels over to the new technology. */ AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) { if (bridge_channel->just_joined) { @@ -1782,11 +1795,11 @@ static int smart_bridge_operation(struct ast_bridge *bridge) */ if (old_technology->destroy) { ast_debug(1, "Bridge %s: deferring %s technology destructor\n", - bridge->uniqueid, old_technology->name); + dummy_bridge.uniqueid, old_technology->name); bridge_queue_action_nodup(bridge, deferred_action); } else { ast_debug(1, "Bridge %s: calling %s technology destructor\n", - bridge->uniqueid, old_technology->name); + dummy_bridge.uniqueid, old_technology->name); ast_module_unref(old_technology->mod); } -- GitLab