[asterisk-commits] mmichelson: branch mmichelson/atxfer_features r392917 - /team/mmichelson/atxf...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Tue Jun 25 18:05:26 CDT 2013
Author: mmichelson
Date: Tue Jun 25 18:05:24 2013
New Revision: 392917
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=392917
Log:
Fix up some referencing issues.
There was a refleak in my move and merge functions since bridge channels
were not being unreffed.
There was also a misunderstanding about ast_bridge_impart's practice of
gobbling the reference of the channel that is being added. This led to
an overabundance of unrefs for the transfer target.
There appears to be a refleak of some sort still, since "core stop now"
after performing a transfer results in a delay.
Modified:
team/mmichelson/atxfer_features/main/bridging_basic.c
Modified: team/mmichelson/atxfer_features/main/bridging_basic.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/atxfer_features/main/bridging_basic.c?view=diff&rev=392917&r1=392916&r2=392917
==============================================================================
--- team/mmichelson/atxfer_features/main/bridging_basic.c (original)
+++ team/mmichelson/atxfer_features/main/bridging_basic.c Tue Jun 25 18:05:24 2013
@@ -31,12 +31,13 @@
ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
+#define REF_DEBUG 1
+#include "asterisk/astobj2.h"
#include "asterisk/channel.h"
#include "asterisk/utils.h"
#include "asterisk/linkedlists.h"
#include "asterisk/bridging.h"
#include "asterisk/bridging_basic.h"
-#include "asterisk/astobj2.h"
#include "asterisk/features_config.h"
#include "asterisk/pbx.h"
#include "asterisk/file.h"
@@ -142,7 +143,6 @@
*/
static int bridge_personality_normal_push(struct ast_bridge *self, struct ast_bridge_channel *bridge_channel, struct ast_bridge_channel *swap)
{
- ast_log(LOG_NOTICE, "NORMAL PUSH\n");
if (ast_bridge_hangup_hook(bridge_channel->features, basic_hangup_hook, NULL, NULL, AST_BRIDGE_HOOK_REMOVE_ON_PULL)
|| ast_bridge_channel_setup_features(bridge_channel)) {
return -1;
@@ -164,6 +164,17 @@
}
return ast_bridge_base_v_table.push(self, bridge_channel, swap);
+}
+
+static void bridge_basic_destroy(struct ast_bridge *self)
+{
+ struct bridge_basic_personality *personality = self->personality;
+
+ ast_assert(personality != NULL);
+
+ ao2_cleanup(personality);
+
+ ast_bridge_base_v_table.destroy(self);
}
static void remove_hooks_on_personality_change(struct ast_bridge *bridge)
@@ -388,6 +399,8 @@
{
struct attended_transfer_properties *props = obj;
+ ast_log(LOG_NOTICE, "What about now? Can I be a happy man again?\n");
+
ao2_cleanup(props->target_bridge);
ao2_cleanup(props->transferee_bridge);
/* Use ao2_cleanup() instead of ast_channel_unref() for channels since they may be NULL */
@@ -434,18 +447,25 @@
return props;
}
-static void attended_transfer_properties_shutdown(struct attended_transfer_properties *props,
- int hangup_target)
+static void clear_stimulus_queue(struct attended_transfer_properties *props)
+{
+ struct stimulus_list *list;
+ SCOPED_MUTEX(lock, &props->lock);
+
+ while ((list = AST_LIST_REMOVE_HEAD(&props->stimulus_queue, next))) {
+ ast_free(list);
+ }
+}
+
+static void attended_transfer_properties_shutdown(struct attended_transfer_properties *props)
{
if (props->transferee_bridge) {
ast_bridge_merge_inhibit(props->transferee_bridge, -1);
+ ast_bridge_basic_change_personality_normal(props->transferee_bridge);
}
if (props->transfer_target) {
SCOPED_CHANNELLOCK(lock, props->transfer_target);
- if (hangup_target) {
- ast_softhangup_nolock(props->transfer_target, AST_SOFTHANGUP_EXPLICIT);
- }
if (props->target_framehook_id != -1) {
ast_framehook_detach(props->transfer_target, props->target_framehook_id);
@@ -454,10 +474,15 @@
if (props->target_bridge) {
ast_bridge_destroy(props->target_bridge);
- }
- /* XXX There will be more here, such as removing transferer role
- * from transferer
- */
+ props->target_bridge = NULL;
+ }
+
+ if (props->transferer) {
+ ast_channel_remove_bridge_role(props->transferer, TRANSFERER_ROLE_NAME);
+ }
+
+ clear_stimulus_queue(props);
+
ao2_cleanup(props);
}
@@ -502,7 +527,7 @@
static int bridge_move(struct ast_bridge *dest, struct ast_bridge *src, struct ast_channel *channel, struct ast_channel *swap)
{
int res;
- struct ast_bridge_channel *bridge_channel;
+ RAII_VAR(struct ast_bridge_channel *, bridge_channel, NULL, ao2_cleanup);
SCOPED_LOCK(lock, dest, ast_bridge_lock, ast_bridge_unlock);
ast_channel_lock(channel);
@@ -521,23 +546,25 @@
static void bridge_merge(struct ast_bridge *dest, struct ast_bridge *src, struct ast_channel **kick_me, unsigned int num_kick)
{
- struct ast_bridge_channel **kick_bridge_channels = NULL;
+ struct ast_bridge_channel **kick_bridge_channels = num_kick ?
+ ast_alloca(num_kick * sizeof(*kick_bridge_channels)) : NULL;
int i;
ast_bridge_lock_both(dest, src);
- if (num_kick) {
- kick_bridge_channels = ast_alloca(num_kick * sizeof(*kick_bridge_channels));
- for (i = 0; i < num_kick; ++i) {
- ast_channel_lock(kick_me[i]);
- kick_bridge_channels[i] = ast_channel_get_bridge_channel(kick_me[i]);
- ast_channel_unlock(kick_me[i]);
- }
+ for (i = 0; i < num_kick; ++i) {
+ ast_channel_lock(kick_me[i]);
+ kick_bridge_channels[i] = ast_channel_get_bridge_channel(kick_me[i]);
+ ast_channel_unlock(kick_me[i]);
}
bridge_merge_do(dest, src, kick_bridge_channels, num_kick);
ast_bridge_unlock(dest);
ast_bridge_unlock(src);
+
+ for (i = 0; i < num_kick; ++i) {
+ ao2_cleanup(kick_bridge_channels[i]);
+ }
}
/*!
@@ -1027,10 +1054,7 @@
const char *swap_dtmf;
struct bridge_basic_personality *personality = self->personality;
- ast_log(LOG_NOTICE, "ATXFER PUSH\n");
-
if (!ast_channel_has_role(bridge_channel->chan, TRANSFERER_ROLE_NAME)) {
- ast_log(LOG_NOTICE, "NOT THE TRANSFERER!\n");
return 0;
}
@@ -1097,7 +1121,6 @@
static void *attended_transfer_monitor_thread(void *data)
{
struct attended_transfer_properties *props = data;
- ast_log(LOG_NOTICE, "ATXFER MONITOR THREAD START\n");
for (;;) {
enum attended_transfer_stimulus stimulus;
@@ -1125,6 +1148,8 @@
ast_log(LOG_NOTICE, "Told to enter state %s next\n", state_properties[props->state].state_name);
}
+
+ attended_transfer_properties_shutdown(props);
return NULL;
}
@@ -1198,7 +1223,7 @@
if (add_transferer_role(props->transferer, attended_transfer)) {
ast_log(LOG_ERROR, "Unable to set transferer bridge role properly\n");
- attended_transfer_properties_shutdown(props, 0);
+ attended_transfer_properties_shutdown(props);
return 0;
}
@@ -1209,7 +1234,7 @@
if (grab_transfer(bridge_channel->chan, props->exten, sizeof(props->exten), props->context)) {
ast_log(LOG_WARNING, "Unable to acquire target extension for attended transfer\n");
ast_bridge_channel_write_unhold(bridge_channel);
- attended_transfer_properties_shutdown(props, 0);
+ attended_transfer_properties_shutdown(props);
return 0;
}
@@ -1224,9 +1249,15 @@
ast_log(LOG_ERROR, "Unable to request outbound channel for attended transfer target\n");
ast_stream_and_wait(props->transferer, props->failsound, AST_DIGIT_NONE);
ast_bridge_channel_write_unhold(bridge_channel);
- attended_transfer_properties_shutdown(props, 0);
+ attended_transfer_properties_shutdown(props);
return 0;
}
+
+ /* We increase the refcount of the transfer target because ast_bridge_impart() will
+ * steal the reference we already have. We need to keep a reference, so the only
+ * choice is to give it a bump
+ */
+ ast_channel_ref(props->transfer_target);
/* Create a bridge to use to talk to the person we are calling */
props->target_bridge = ast_bridge_basic_new();
@@ -1234,7 +1265,9 @@
ast_log(LOG_ERROR, "Unable to create bridge for attended transfer target\n");
ast_stream_and_wait(props->transferer, props->failsound, AST_DIGIT_NONE);
ast_bridge_channel_write_unhold(bridge_channel);
- attended_transfer_properties_shutdown(props, 1);
+ ast_hangup(props->transfer_target);
+ props->transfer_target = NULL;
+ attended_transfer_properties_shutdown(props);
return 0;
}
ast_bridge_merge_inhibit(props->target_bridge, +1);
@@ -1243,7 +1276,9 @@
ast_log(LOG_ERROR, "Unable to attach framehook to transfer target\n");
ast_stream_and_wait(props->transferer, props->failsound, AST_DIGIT_NONE);
ast_bridge_channel_write_unhold(bridge_channel);
- attended_transfer_properties_shutdown(props, 1);
+ ast_hangup(props->transfer_target);
+ props->transfer_target = NULL;
+ attended_transfer_properties_shutdown(props);
return 0;
}
@@ -1254,16 +1289,23 @@
ast_log(LOG_ERROR, "Unable to place outbound call to transfer target\n");
ast_stream_and_wait(bridge_channel->chan, props->failsound, AST_DIGIT_NONE);
ast_bridge_channel_write_unhold(bridge_channel);
- attended_transfer_properties_shutdown(props, 1);
+ ast_hangup(props->transfer_target);
+ props->transfer_target = NULL;
+ attended_transfer_properties_shutdown(props);
return 0;
}
- /* This is how this is going down, we are imparting the channel we called above into this bridge first */
+ /* We increase the refcount of the transfer target because ast_bridge_impart() will
+ * steal the reference we already have. We need to keep a reference, so the only
+ * choice is to give it a bump
+ */
+ ast_channel_ref(props->transfer_target);
if (ast_bridge_impart(props->target_bridge, props->transfer_target, NULL, NULL, 1)) {
ast_log(LOG_ERROR, "Unable to place transfer target into bridge\n");
ast_stream_and_wait(bridge_channel->chan, props->failsound, AST_DIGIT_NONE);
ast_bridge_channel_write_unhold(bridge_channel);
- attended_transfer_properties_shutdown(props, 1);
+ ast_hangup(props->transfer_target);
+ attended_transfer_properties_shutdown(props);
return 0;
}
@@ -1271,7 +1313,7 @@
ast_log(LOG_ERROR, "Unable to create monitoring thread for attended transfer\n");
ast_stream_and_wait(bridge_channel->chan, props->failsound, AST_DIGIT_NONE);
ast_bridge_channel_write_unhold(bridge_channel);
- attended_transfer_properties_shutdown(props, 1);
+ attended_transfer_properties_shutdown(props);
return 0;
}
@@ -1373,6 +1415,7 @@
ast_bridge_basic_v_table = ast_bridge_base_v_table;
ast_bridge_basic_v_table.name = "basic";
ast_bridge_basic_v_table.push = bridge_basic_push;
+ ast_bridge_basic_v_table.destroy = bridge_basic_destroy;
personality_normal_v_table = ast_bridge_base_v_table;
personality_normal_v_table.name = "normal";
More information about the asterisk-commits
mailing list