Asterisk
/
asterisk
mirror of https://github.com/asterisk/asterisk
1
0
Fork 0
Code Issues Releases Wiki Activity

Improved feature limits interval hook implementaion.

Browse Source 
* Fixed feature limits to not use special members of struct
ast_bridge_features.

* Fixed memory leak in off nominal paths of bridge_builtin_set_limits().

* Fixed off nominal path in ast_bridge_features_limits_construct() freeing
unallocated memory if it was not called by bridge_builtin_set_limits().

* Made bridge_builtin_interval_features.so unloadable.

* Simplified parking's use of its duration interval hook.

* Made BridgeWait S option not depend upon another module being loaded.

(closes issue ASTERISK-22107)
Reported by: Matt Jordan

Review: https://reviewboard.asterisk.org/r/2701/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@395559 65c4cc65-6c06-0410-ace0-fbb531ad65f3
changes/78/78/1
Richard Mudgett 12 years ago
parent b6f9b39830
commit 50aba6be36
5 changed files with 68 additions and 86 deletions
Show Stats Download Patch File Download Diff File

30
apps/app_bridgewait.c
Unescape View File

@ -201,32 +201,34 @@ AST_APP_OPTIONS(bridgewait_opts, {
AST_APP_OPTION_ARG('S', MUXFLAG_TIMEOUT, OPT_ARG_TIMEOUT), AST_APP_OPTION_ARG('S', MUXFLAG_TIMEOUT, OPT_ARG_TIMEOUT),
}); });
static int bridgewait_timeout_callback(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, void *hook_pvt)
{
ast_verb(3, "Channel %s timed out.\n", ast_channel_name(bridge_channel->chan));
ast_bridge_channel_leave_bridge(bridge_channel, BRIDGE_CHANNEL_STATE_END);
return -1;
}
static int apply_option_timeout(struct ast_bridge_features *features, char *duration_arg) static int apply_option_timeout(struct ast_bridge_features *features, char *duration_arg)
{ {
struct ast_bridge_features_limits hold_limits; unsigned int duration;
if (ast_strlen_zero(duration_arg)) { if (ast_strlen_zero(duration_arg)) {
ast_log(LOG_ERROR, "No duration value provided for the timeout ('S') option.\n"); ast_log(LOG_ERROR, "Timeout option 'S': No value provided.\n");
return -1; return -1;
} }
if (sscanf(duration_arg, "%u", &duration) != 1 || duration == 0) {
if (ast_bridge_features_limits_construct(&hold_limits)) { ast_log(LOG_ERROR, "Timeout option 'S': Invalid value provided '%s'.\n",
ast_log(LOG_ERROR, "Could not construct duration limits. Bridge canceled.\n"); duration_arg);
return -1; return -1;
} }
if (sscanf(duration_arg, "%u", &hold_limits.duration) != 1 duration *= 1000;
|| hold_limits.duration == 0) { if (ast_bridge_interval_hook(features, duration, bridgewait_timeout_callback,
ast_log(LOG_ERROR, "Duration value provided for the timeout ('S') option must be greater than 0\n"); NULL, NULL, AST_BRIDGE_HOOK_REMOVE_ON_PULL)) {
ast_bridge_features_limits_destroy(&hold_limits); ast_log(LOG_ERROR, "Timeout option 'S': Could not create timer.\n");
return -1; return -1;
} }
/* Limits struct holds time as milliseconds, so muliply 1000x */
hold_limits.duration *= 1000;
ast_bridge_features_set_limits(features, &hold_limits, AST_BRIDGE_HOOK_REMOVE_ON_PULL);
ast_bridge_features_limits_destroy(&hold_limits);
return 0; return 0;
} }

84
bridges/bridge_builtin_interval_features.c
Unescape View File

@ -64,7 +64,7 @@ static int bridge_features_duration_callback(struct ast_bridge *bridge, struct a
return -1; return -1;
} }
static void limits_interval_playback(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_bridge_features_limits *limits, const char *file) static void limits_interval_playback(struct ast_bridge_channel *bridge_channel, struct ast_bridge_features_limits *limits, const char *file)
{ {
if (!strcasecmp(file, "timeleft")) { if (!strcasecmp(file, "timeleft")) {
unsigned int remaining = ast_tvdiff_ms(limits->quitting_time, ast_tvnow()) / 1000; unsigned int remaining = ast_tvdiff_ms(limits->quitting_time, ast_tvnow()) / 1000;
@ -114,11 +114,7 @@ static int bridge_features_connect_callback(struct ast_bridge *bridge, struct as
{ {
struct ast_bridge_features_limits *limits = hook_pvt; struct ast_bridge_features_limits *limits = hook_pvt;
if (bridge_channel->state != BRIDGE_CHANNEL_STATE_WAIT) { limits_interval_playback(bridge_channel, limits, limits->connect_sound);
return -1;
}
limits_interval_playback(bridge, bridge_channel, limits, limits->connect_sound);
return -1; return -1;
} }
@ -126,72 +122,75 @@ static int bridge_features_warning_callback(struct ast_bridge *bridge, struct as
{ {
struct ast_bridge_features_limits *limits = hook_pvt; struct ast_bridge_features_limits *limits = hook_pvt;
if (bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT) { limits_interval_playback(bridge_channel, limits, limits->warning_sound);
/* If we aren't in the wait state, something more important than this warning is happening and we should skip it. */ return limits->frequency ?: -1;
limits_interval_playback(bridge, bridge_channel, limits, limits->warning_sound);
} }
return !limits->frequency ? -1 : limits->frequency; static void bridge_features_limits_copy(struct ast_bridge_features_limits *dst, struct ast_bridge_features_limits *src)
}
static void copy_bridge_features_limits(struct ast_bridge_features_limits *dst, struct ast_bridge_features_limits *src)
{ {
ast_string_fields_copy(dst, src);
dst->quitting_time = src->quitting_time;
dst->duration = src->duration; dst->duration = src->duration;
dst->warning = src->warning; dst->warning = src->warning;
dst->frequency = src->frequency; dst->frequency = src->frequency;
dst->quitting_time = src->quitting_time; }
ast_string_field_set(dst, duration_sound, src->duration_sound); static void bridge_features_limits_dtor(void *vdoomed)
ast_string_field_set(dst, warning_sound, src->warning_sound); {
ast_string_field_set(dst, connect_sound, src->connect_sound); struct ast_bridge_features_limits *doomed = vdoomed;
ast_bridge_features_limits_destroy(doomed);
ast_module_unref(ast_module_info->self);
} }
static int bridge_builtin_set_limits(struct ast_bridge_features *features, static int bridge_builtin_set_limits(struct ast_bridge_features *features,
struct ast_bridge_features_limits *limits, enum ast_bridge_hook_remove_flags remove_flags) struct ast_bridge_features_limits *limits,
enum ast_bridge_hook_remove_flags remove_flags)
{ {
struct ast_bridge_features_limits *feature_limits; RAII_VAR(struct ast_bridge_features_limits *, feature_limits, NULL, ao2_cleanup);
if (!limits->duration) { if (!limits->duration) {
return -1; return -1;
} }
if (features->limits) { /* Create limits hook_pvt data. */
ast_log(LOG_ERROR, "Tried to apply limits to a feature set that already has limits.\n"); ast_module_ref(ast_module_info->self);
return -1; feature_limits = ao2_alloc_options(sizeof(*feature_limits),
} bridge_features_limits_dtor, AO2_ALLOC_OPT_LOCK_NOLOCK);
feature_limits = ast_malloc(sizeof(*feature_limits));
if (!feature_limits) { if (!feature_limits) {
ast_module_unref(ast_module_info->self);
return -1; return -1;
} }
if (ast_bridge_features_limits_construct(feature_limits)) { if (ast_bridge_features_limits_construct(feature_limits)) {
return -1; return -1;
} }
bridge_features_limits_copy(feature_limits, limits);
feature_limits->quitting_time = ast_tvadd(ast_tvnow(),
ast_samp2tv(feature_limits->duration, 1000));
copy_bridge_features_limits(feature_limits, limits); /* Install limit hooks. */
features->limits = feature_limits; ao2_ref(feature_limits, +1);
/* BUGBUG feature interval hooks need to be reimplemented to be more stand alone. */
if (ast_bridge_interval_hook(features, feature_limits->duration, if (ast_bridge_interval_hook(features, feature_limits->duration,
bridge_features_duration_callback, feature_limits, NULL, remove_flags)) { bridge_features_duration_callback, feature_limits, __ao2_cleanup, remove_flags)) {
ast_log(LOG_ERROR, "Failed to schedule the duration limiter to the bridge channel.\n"); ast_log(LOG_ERROR, "Failed to schedule the duration limiter to the bridge channel.\n");
ao2_ref(feature_limits, -1);
return -1; return -1;
} }
feature_limits->quitting_time = ast_tvadd(ast_tvnow(), ast_samp2tv(feature_limits->duration, 1000));
if (!ast_strlen_zero(feature_limits->connect_sound)) { if (!ast_strlen_zero(feature_limits->connect_sound)) {
ao2_ref(feature_limits, +1);
if (ast_bridge_interval_hook(features, 1, if (ast_bridge_interval_hook(features, 1,
bridge_features_connect_callback, feature_limits, NULL, remove_flags)) { bridge_features_connect_callback, feature_limits, __ao2_cleanup, remove_flags)) {
ast_log(LOG_WARNING, "Failed to schedule connect sound to the bridge channel.\n"); ast_log(LOG_WARNING, "Failed to schedule connect sound to the bridge channel.\n");
ao2_ref(feature_limits, -1);
} }
} }
if (feature_limits->warning && feature_limits->warning < feature_limits->duration) { if (feature_limits->warning && feature_limits->warning < feature_limits->duration) {
if (ast_bridge_interval_hook(features, feature_limits->duration - feature_limits->warning, ao2_ref(feature_limits, +1);
bridge_features_warning_callback, feature_limits, NULL, remove_flags)) { if (ast_bridge_interval_hook(features,
feature_limits->duration - feature_limits->warning,
bridge_features_warning_callback, feature_limits, __ao2_cleanup, remove_flags)) {
ast_log(LOG_WARNING, "Failed to schedule warning sound playback to the bridge channel.\n"); ast_log(LOG_WARNING, "Failed to schedule warning sound playback to the bridge channel.\n");
ao2_ref(feature_limits, -1);
} }
} }
@ -200,17 +199,14 @@ static int bridge_builtin_set_limits(struct ast_bridge_features *features,
static int unload_module(void) static int unload_module(void)
{ {
ast_bridge_interval_unregister(AST_BRIDGE_BUILTIN_INTERVAL_LIMITS);
return 0; return 0;
} }
static int load_module(void) static int load_module(void)
{ {
ast_bridge_interval_register(AST_BRIDGE_BUILTIN_INTERVAL_LIMITS, bridge_builtin_set_limits); return ast_bridge_interval_register(AST_BRIDGE_BUILTIN_INTERVAL_LIMITS,
bridge_builtin_set_limits);
/* Bump up our reference count so we can't be unloaded. */
ast_module_ref(ast_module_info->self);
return AST_MODULE_LOAD_SUCCESS;
} }
AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Built in bridging interval features"); AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Built in bridging interval features");

15
include/asterisk/bridge_features.h
Unescape View File

@ -237,8 +237,6 @@ struct ast_bridge_features {
struct ao2_container *other_hooks; struct ao2_container *other_hooks;
/*! Attached interval hooks */ /*! Attached interval hooks */
struct ast_heap *interval_hooks; struct ast_heap *interval_hooks;
/*! Limits feature data */
struct ast_bridge_features_limits *limits;
/*! Feature flags that are enabled */ /*! Feature flags that are enabled */
struct ast_flags feature_flags; struct ast_flags feature_flags;
/*! Used to assign the sequence number to the next interval hook added. */ /*! Used to assign the sequence number to the next interval hook added. */
@ -298,13 +296,6 @@ struct ast_bridge_features_automixmonitor {
* \brief Structure that contains configuration information for the limits feature * \brief Structure that contains configuration information for the limits feature
*/ */
struct ast_bridge_features_limits { struct ast_bridge_features_limits {
/*! Maximum duration that the channel is allowed to be in the bridge (specified in milliseconds) */
unsigned int duration;
/*! Duration into the call when warnings should begin (specified in milliseconds or 0 to disable) */
unsigned int warning;
/*! Interval between the warnings (specified in milliseconds or 0 to disable) */
unsigned int frequency;
AST_DECLARE_STRING_FIELDS( AST_DECLARE_STRING_FIELDS(
/*! Sound file to play when the maximum duration is reached (if empty, then nothing will be played) */ /*! Sound file to play when the maximum duration is reached (if empty, then nothing will be played) */
AST_STRING_FIELD(duration_sound); AST_STRING_FIELD(duration_sound);
@ -315,6 +306,12 @@ struct ast_bridge_features_limits {
); );
/*! Time when the bridge will be terminated by the limits feature */ /*! Time when the bridge will be terminated by the limits feature */
struct timeval quitting_time; struct timeval quitting_time;
/*! Maximum duration that the channel is allowed to be in the bridge (specified in milliseconds) */
unsigned int duration;
/*! Duration into the call when warnings should begin (specified in milliseconds or 0 to disable) */
unsigned int warning;
/*! Interval between the warnings (specified in milliseconds or 0 to disable) */
unsigned int frequency;
}; };
/*! /*!

17
main/bridge.c
Unescape View File

@ -2974,7 +2974,6 @@ int ast_bridge_features_limits_construct(struct ast_bridge_features_limits *limi
memset(limits, 0, sizeof(*limits)); memset(limits, 0, sizeof(*limits));
if (ast_string_field_init(limits, 256)) { if (ast_string_field_init(limits, 256)) {
ast_free(limits);
return -1; return -1;
} }
@ -2987,13 +2986,14 @@ void ast_bridge_features_limits_destroy(struct ast_bridge_features_limits *limit
} }
int ast_bridge_features_set_limits(struct ast_bridge_features *features, int ast_bridge_features_set_limits(struct ast_bridge_features *features,
struct ast_bridge_features_limits *limits, enum ast_bridge_hook_remove_flags remove_flags) struct ast_bridge_features_limits *limits,
enum ast_bridge_hook_remove_flags remove_flags)
{ {
if (builtin_interval_handlers[AST_BRIDGE_BUILTIN_INTERVAL_LIMITS]) { if (builtin_interval_handlers[AST_BRIDGE_BUILTIN_INTERVAL_LIMITS]) {
ast_bridge_builtin_set_limits_fn bridge_features_set_limits_callback; ast_bridge_builtin_set_limits_fn callback;
bridge_features_set_limits_callback = builtin_interval_handlers[AST_BRIDGE_BUILTIN_INTERVAL_LIMITS]; callback = builtin_interval_handlers[AST_BRIDGE_BUILTIN_INTERVAL_LIMITS];
return bridge_features_set_limits_callback(features, limits, remove_flags); return callback(features, limits, remove_flags);
} }
ast_log(LOG_ERROR, "Attempted to set limits without an AST_BRIDGE_BUILTIN_INTERVAL_LIMITS callback registered.\n"); ast_log(LOG_ERROR, "Attempted to set limits without an AST_BRIDGE_BUILTIN_INTERVAL_LIMITS callback registered.\n");
@ -3181,13 +3181,6 @@ void ast_bridge_features_cleanup(struct ast_bridge_features *features)
features->interval_hooks = ast_heap_destroy(features->interval_hooks); features->interval_hooks = ast_heap_destroy(features->interval_hooks);
} }
/* If the features contains a limits pvt, destroy that as well. */
if (features->limits) {
ast_bridge_features_limits_destroy(features->limits);
ast_free(features->limits);
features->limits = NULL;
}
/* Destroy the miscellaneous other hooks container. */ /* Destroy the miscellaneous other hooks container. */
ao2_cleanup(features->other_hooks); ao2_cleanup(features->other_hooks);
features->other_hooks = NULL; features->other_hooks = NULL;

8
res/parking/parking_bridge_features.c
Unescape View File

@ -363,12 +363,6 @@ static int feature_park(struct ast_bridge *bridge, struct ast_bridge_channel *br
return 0; return 0;
} }
static void parking_duration_cb_destroyer(void *hook_pvt)
{
struct parked_user *user = hook_pvt;
ao2_ref(user, -1);
}
/*! \internal /*! \internal
* \brief Interval hook. Pulls a parked call from the parking bridge after the timeout is passed and sets the resolution to timeout. * \brief Interval hook. Pulls a parked call from the parking bridge after the timeout is passed and sets the resolution to timeout.
* *
@ -523,7 +517,7 @@ void parking_set_duration(struct ast_bridge_features *features, struct parked_us
ao2_ref(user, +1); ao2_ref(user, +1);
if (ast_bridge_interval_hook(features, time_limit, if (ast_bridge_interval_hook(features, time_limit,
parking_duration_callback, user, parking_duration_cb_destroyer, AST_BRIDGE_HOOK_REMOVE_ON_PULL)) { parking_duration_callback, user, __ao2_cleanup, AST_BRIDGE_HOOK_REMOVE_ON_PULL)) {
ast_log(LOG_ERROR, "Failed to apply duration limits to the parking call.\n"); ast_log(LOG_ERROR, "Failed to apply duration limits to the parking call.\n");
ao2_ref(user, -1); ao2_ref(user, -1);
} }

Write Preview
Loading…
Cancel
Save