[asterisk-commits] bridge softmix.c: Fix crash if channel fails to join mixing ... (asterisk[master])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Apr 25 21:08:15 CDT 2016
Anonymous Coward #1000019 has submitted this change and it was merged.
Change subject: bridge_softmix.c: Fix crash if channel fails to join mixing tech.
......................................................................
bridge_softmix.c: Fix crash if channel fails to join mixing tech.
softmix_bridge_join() failed because of an allocation failure. To address
this, the softmix bridge technology now checks if the channel failed to
join softmix successfully. In addition, the bridge now begins the process
of kicking the channel out of the bridge so we don't have channels
partially in the bridge for very long.
* Fix the test_channel_feature_hooks.c unit tests. The test channel must
have a valid codec to join the simple_bridge technology. This patch makes
joining a bridge more strict by not allowing partially joined channels to
remain in the bridge.
Change-Id: I97e2ade6a2bcd1214f24fb839fda948825b61a2b
---
M bridges/bridge_softmix.c
M include/asterisk/bridge_technology.h
M main/bridge.c
M tests/test_channel_feature_hooks.c
4 files changed, 32 insertions(+), 3 deletions(-)
Approvals:
Mark Michelson: Looks good to me, but someone else must approve
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index bad3e43..0991e28 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -359,6 +359,9 @@
struct ast_format *slin_format;
int setup_fail;
+ /* The callers have already ensured that sc is never NULL. */
+ ast_assert(sc != NULL);
+
slin_format = ast_format_cache_get_slin_by_rate(rate);
ast_mutex_lock(&sc->lock);
@@ -714,7 +717,7 @@
{
int res = 0;
- if (!bridge->tech_pvt || (bridge_channel && !bridge_channel->tech_pvt)) {
+ if (!bridge->tech_pvt || !bridge_channel || !bridge_channel->tech_pvt) {
/* "Accept" the frame and discard it. */
return 0;
}
@@ -984,6 +987,11 @@
AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) {
struct softmix_channel *sc = bridge_channel->tech_pvt;
+ if (!sc) {
+ /* This channel failed to join successfully. */
+ continue;
+ }
+
/* Update the sample rate to match the bridge's native sample rate if necessary. */
if (update_all_rates) {
set_softmix_bridge_data(softmix_data->internal_rate, softmix_data->internal_mixing_interval, bridge_channel, 1);
@@ -1019,7 +1027,8 @@
AST_LIST_TRAVERSE(&bridge->channels, bridge_channel, entry) {
struct softmix_channel *sc = bridge_channel->tech_pvt;
- if (bridge_channel->suspended) {
+ if (!sc || bridge_channel->suspended) {
+ /* This channel failed to join successfully or is suspended. */
continue;
}
diff --git a/include/asterisk/bridge_technology.h b/include/asterisk/bridge_technology.h
index b83a51b..8df19d9 100644
--- a/include/asterisk/bridge_technology.h
+++ b/include/asterisk/bridge_technology.h
@@ -107,6 +107,9 @@
* \retval -1 on failure
*
* \note On entry, bridge is already locked.
+ *
+ * \note The bridge technology must tollerate a failed to join channel
+ * until it can be kicked from the bridge.
*/
int (*join)(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
/*!
diff --git a/main/bridge.c b/main/bridge.c
index b9a436e..a56555b 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -420,10 +420,12 @@
bridge->technology->name);
if (bridge->technology->join
&& bridge->technology->join(bridge, bridge_channel)) {
- ast_debug(1, "Bridge %s: %p(%s) failed to join %s technology\n",
+ /* We cannot leave the channel partially in the bridge so we must kick it out */
+ ast_debug(1, "Bridge %s: %p(%s) failed to join %s technology (Kicking it out)\n",
bridge->uniqueid, bridge_channel, ast_channel_name(bridge_channel->chan),
bridge->technology->name);
bridge_channel->just_joined = 1;
+ ast_bridge_channel_leave_bridge(bridge_channel, BRIDGE_CHANNEL_STATE_END, 0);
return;
}
diff --git a/tests/test_channel_feature_hooks.c b/tests/test_channel_feature_hooks.c
index ad72688..94037e2 100644
--- a/tests/test_channel_feature_hooks.c
+++ b/tests/test_channel_feature_hooks.c
@@ -40,12 +40,15 @@
#include "asterisk/bridge.h"
#include "asterisk/bridge_basic.h"
#include "asterisk/features.h"
+#include "asterisk/format_cache.h"
#define TEST_CATEGORY "/channels/features/"
#define CHANNEL_TECH_NAME "FeaturesTestChannel"
#define TEST_BACKEND_NAME "Features Test Logging"
+
+#define TEST_CHANNEL_FORMAT ast_format_slin
/*! \brief A channel technology used for the unit tests */
static struct ast_channel_tech test_features_chan_tech = {
@@ -94,6 +97,11 @@
#define START_CHANNEL(channel, name, number) do { \
channel = ast_channel_alloc(0, AST_STATE_UP, number, name, number, number, \
"default", NULL, NULL, 0, CHANNEL_TECH_NAME "/" name); \
+ ast_channel_nativeformats_set(channel, test_features_chan_tech.capabilities); \
+ ast_channel_set_rawwriteformat(channel, TEST_CHANNEL_FORMAT); \
+ ast_channel_set_rawreadformat(channel, TEST_CHANNEL_FORMAT); \
+ ast_channel_set_writeformat(channel, TEST_CHANNEL_FORMAT); \
+ ast_channel_set_readformat(channel, TEST_CHANNEL_FORMAT); \
ast_channel_unlock(channel); \
} while (0)
@@ -329,12 +337,19 @@
AST_TEST_UNREGISTER(test_features_channel_interval);
ast_channel_unregister(&test_features_chan_tech);
+ ao2_cleanup(test_features_chan_tech.capabilities);
+ test_features_chan_tech.capabilities = NULL;
return 0;
}
static int load_module(void)
{
+ test_features_chan_tech.capabilities = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
+ if (!test_features_chan_tech.capabilities) {
+ return AST_MODULE_LOAD_FAILURE;
+ }
+ ast_format_cap_append(test_features_chan_tech.capabilities, TEST_CHANNEL_FORMAT, 0);
ast_channel_register(&test_features_chan_tech);
AST_TEST_REGISTER(test_features_channel_dtmf);
--
To view, visit https://gerrit.asterisk.org/2589
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I97e2ade6a2bcd1214f24fb839fda948825b61a2b
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-commits
mailing list