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"; -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits