This commit is contained in:
Mark Murawski 2024-03-20 15:22:28 +02:00 committed by GitHub
commit 0cb8205817
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 206 additions and 40 deletions

View File

@ -137,14 +137,20 @@ struct pickup_by_name_args {
static int find_by_name(void *obj, void *arg, void *data, int flags)
{
struct ast_channel *target = obj;/*!< Potential pickup target */
struct pickup_by_name_args *args = data;
struct ast_channel_callback_data *callback_data = data;
struct pickup_by_name_args *args = callback_data->data_additional;
if (args->chan == target) {
/* The channel attempting to pickup a call cannot pickup itself. */
return 0;
}
ast_channel_lock(target);
if (ast_channel_trylock(target) != 0) {
/* Lock failure means the caller will need to retry until we get the lock we need */
callback_data->final_match_result = CMP_RETRY_NEEDED;
return CMP_STOP;
}
if (!strncasecmp(ast_channel_name(target), args->name, args->len)
&& ast_can_pickup(target)) {
/* Return with the channel still locked on purpose */
@ -158,14 +164,20 @@ static int find_by_name(void *obj, void *arg, void *data, int flags)
static int find_by_uniqueid(void *obj, void *arg, void *data, int flags)
{
struct ast_channel *target = obj;/*!< Potential pickup target */
struct pickup_by_name_args *args = data;
struct ast_channel_callback_data *callback_data = data;
struct pickup_by_name_args *args = callback_data->data_additional;
if (args->chan == target) {
/* The channel attempting to pickup a call cannot pickup itself. */
return 0;
}
ast_channel_lock(target);
if (ast_channel_trylock(target) != 0) {
/* Lock failure means the caller will need to retry until we get the lock we need */
callback_data->final_match_result = CMP_RETRY_NEEDED;
return CMP_STOP;
}
if (!strcasecmp(ast_channel_uniqueid(target), args->name)
&& ast_can_pickup(target)) {
/* Return with the channel still locked on purpose */
@ -205,7 +217,7 @@ static struct ast_channel *find_by_channel(struct ast_channel *chan, const char
strcat(chkchan, "-");
pickup_args.name = chkchan;
}
target = ast_channel_callback(find_by_name, NULL, &pickup_args, 0);
target = ast_channel_callback_safe(find_by_name, "channel name, where pickup is possible", pickup_args.name, NULL, &pickup_args, 0);
if (target) {
return target;
}
@ -213,7 +225,8 @@ static struct ast_channel *find_by_channel(struct ast_channel *chan, const char
/* Now try a search for uniqueid. */
pickup_args.name = channame;
pickup_args.len = 0;
return ast_channel_callback(find_by_uniqueid, NULL, &pickup_args, 0);
return ast_channel_callback_safe(find_by_uniqueid, "channel uniqueid, where pickup is possible", pickup_args.name, NULL, &pickup_args, 0);
}
/*! \brief Attempt to pick up named channel. */
@ -380,7 +393,9 @@ static struct ast_channel *find_by_part(struct ast_channel *chan, const char *pa
/* Try a partial channel name search. */
pickup_args.name = part;
pickup_args.len = strlen(part);
target = ast_channel_callback(find_by_name, NULL, &pickup_args, 0);
target = ast_channel_callback_safe(find_by_name, "channel prefix, where pickup is possible", pickup_args.name, NULL, &pickup_args, 0);
if (target) {
return target;
}
@ -388,7 +403,8 @@ static struct ast_channel *find_by_part(struct ast_channel *chan, const char *pa
/* Now try a search for uniqueid. */
pickup_args.name = part;
pickup_args.len = 0;
return ast_channel_callback(find_by_uniqueid, NULL, &pickup_args, 0);
return ast_channel_callback_safe(find_by_uniqueid, "channel uniqueid prefix, where pickup is possible", pickup_args.name, NULL, &pickup_args, 0);
}
/* Attempt to pick up specified by partial channel name */

View File

@ -973,6 +973,7 @@ Operations on container include:
0: no match, keep going
CMP_STOP: stop search, no match
CMP_MATCH: This object is matched.
CMP_RETRY_NEEDED: Stop search, no match, but we ran into a possible deadlock and the search should be tried again
Note that the entire operation is run with the container
locked, so nobody else can change its content while we work on it.
@ -1024,8 +1025,9 @@ to define callback and hash functions and their arguments.
* The latter will terminate the search in a container.
*/
enum _cb_results {
CMP_MATCH = 0x1, /*!< the object matches the request */
CMP_STOP = 0x2, /*!< stop the search now */
CMP_MATCH = 0x1, /*!< the object matches the request */
CMP_STOP = 0x2, /*!< stop the search now */
CMP_RETRY_NEEDED = 0x4, /*!< stop the search now, unlock the container, rollback so we can retry */
};
/*!

View File

@ -1181,6 +1181,12 @@ enum ama_flags {
AST_AMA_DOCUMENTATION,
};
/* For multi-value data passed to/from ast_channel_callback() (and then into subsequent cb_fn calls) via ast_channel_callback_safe() */
struct ast_channel_callback_data {
void *data_additional;
int final_match_result;
};
/*!
* \note None of the datastore API calls lock the ast_channel they are using.
* So, the channel should be locked before calling the functions that
@ -3076,14 +3082,49 @@ struct ast_channel *ast_channel_iterator_next(struct ast_channel_iterator *i);
*
* \details
* This function executes a callback one time for each active channel on the
* system. The channel is provided as an argument to the function.
* system. The channel (or thing) to be searched for is provided as an argument (arg)
* to the function.
*
* \param arg Must be a char *, Typically either a channel name or a channel uniqueid.
* In some cases exten@context is used.
* Will be passed to cb_fn as its arg param
* \param data Arbitrary data to be passed to cb_fn as its data param
*
* \note This function SHOULD NOT be called unless fully understanding the implications.
* \note Absolutely _NO_ channel locks should be held before calling this function.
* \see ast_channel_callback_safe
* \since 1.8
*/
struct ast_channel *ast_channel_callback(ao2_callback_data_fn *cb_fn, void *arg,
void *data, int ao2_flags);
/*!
* \brief Call a function with every active channel while avoiding deadlocks
*
* \details
* This function executes a callback one time for each active channel on the
* system. The channel (or thing) to be searched for is provided as an argument (arg)
* to the function.
*
* In order to avoid deadlocks:
* cb_fn is required to trylock on any channel(s) it needs and immediately abort
* searching if we cannot get immediately get a lock on said channel.
* If a lock cannot be aquired, CMP_RETRY_NEEDED must be set prior to stopping
*
* \see ast_channel_by_name_cb for reference implementation details
*
* \param arg Must be a char *, Typically either a channel name or a channel uniqueid.
* In some cases exten@context is used.
* Will be passed to cb_fn as its arg param
* \param data Arbitrary data to be passed to cb_fn as its data_additional member
*
* \note Absolutely _NO_ channel locks should be held before calling this function.
* \since 18.20
*/
struct ast_channel *ast_channel_callback_safe(ao2_callback_data_fn *cb_fn,
const char *search_type, const char *search_type_data,
const char *search, void *data_additional, int ao2_flags);
/*! @{ Channel search functions */
/*!

View File

@ -708,16 +708,75 @@ static void ast_channel_destructor(void *obj);
static void ast_dummy_channel_destructor(void *obj);
static int ast_channel_by_uniqueid_cb(void *obj, void *arg, void *data, int flags);
static struct ast_channel *ast_channel_callback_internal(ao2_callback_data_fn *cb_fn, void *arg,
void *data, int ao2_flags)
{
return ao2_callback_data(channels, ao2_flags, cb_fn, arg, data);
}
struct ast_channel *ast_channel_callback(ao2_callback_data_fn *cb_fn, void *arg,
void *data, int ao2_flags)
{
ast_log(LOG_WARNING, "Using ast_channel_callback() directly may lead to deadlocks. Please update your module.\n");
return ast_channel_callback_internal(cb_fn, arg, data, ao2_flags);
}
struct ast_channel *ast_channel_callback_safe(ao2_callback_data_fn *cb_fn, const char *search_type, const char *search_type_data, const char *search, void *data_additional, int ao2_flags)
{
struct ast_channel *channel = NULL;
char *l_search = (char *) search;
struct ast_channel_callback_data callback_data = {
.data_additional = data_additional,
.final_match_result = 0,
};
if (ast_strlen_zero(search)) {
return NULL;
}
ast_debug(5, "Try to find channel by %s: %s\n", search_type, search_type_data);
for (;;) {
callback_data.final_match_result = 0;
/* It's REQUIRED that the passed in cb_fn does a trylock on the channel it wants to operate on, and *
* then if the trylock fails: stop the search and set callback_data.final_match_result as necessary */
channel = ast_channel_callback_internal(cb_fn, l_search, &callback_data, ao2_flags);
/* In order to avoid deadlocks, ast_channel_callback must operate quickly and without holding *
* locks for any length of time longer than necessary to return a channel pointer */
/* ast_channel_callback will lock the 'channels' (main channel list) *
* mutex. We don't want to hold onto the channels mutex AND block *
* on a channel lock, if we cannot grab a channel lock right away */
if (callback_data.final_match_result == CMP_RETRY_NEEDED) {
/* We ran into a possible deadlock, try again */
ast_debug(3, "Avoid deadlock trying to find channel by %s: %s\n", search_type, search_type_data);
usleep(100);
continue;
}
/* We found a match or the search fully completed through all channels */
break;
}
return channel;
}
static int does_id_conflict(const char *uniqueid)
{
struct ast_channel *conflict;
char *l_uniqueid = (char *) uniqueid;
size_t length = 0;
if (ast_strlen_zero(uniqueid)) {
return 0;
}
conflict = ast_channel_callback(ast_channel_by_uniqueid_cb, (char *) uniqueid, &length, OBJ_NOLOCK);
conflict = ast_channel_callback_safe(ast_channel_by_uniqueid_cb, "channel uniqueid", uniqueid, l_uniqueid, &length, OBJ_NOLOCK);
if (conflict) {
ast_log(LOG_ERROR, "Channel Unique ID '%s' already in use by channel %s(%p)\n",
uniqueid, ast_channel_name(conflict), conflict);
@ -1275,17 +1334,12 @@ void ast_channel_undefer_dtmf(struct ast_channel *chan)
}
}
struct ast_channel *ast_channel_callback(ao2_callback_data_fn *cb_fn, void *arg,
void *data, int ao2_flags)
{
return ao2_callback_data(channels, ao2_flags, cb_fn, arg, data);
}
static int ast_channel_by_name_cb(void *obj, void *arg, void *data, int flags)
{
struct ast_channel *chan = obj;
const char *name = arg;
size_t name_len = *(size_t *) data;
struct ast_channel_callback_data *callback_data = data;
const char *name = arg;
size_t name_len = *(size_t *) callback_data->data_additional;
int ret = CMP_MATCH;
if (ast_strlen_zero(name)) {
@ -1293,21 +1347,30 @@ static int ast_channel_by_name_cb(void *obj, void *arg, void *data, int flags)
return CMP_STOP;
}
ast_channel_lock(chan);
if (ast_channel_trylock(chan) != 0) {
ast_debug(2, "Deadlock-Avoid RETRY NEEDED for: %s\n", name);
/* Lock failure means the caller will need to retry until we get the lock we need */
callback_data->final_match_result = CMP_RETRY_NEEDED;
return CMP_STOP;
}
if ((!name_len && strcasecmp(ast_channel_name(chan), name))
|| (name_len && strncasecmp(ast_channel_name(chan), name, name_len))) {
ret = 0; /* name match failed, keep looking */
}
ast_channel_unlock(chan);
callback_data->final_match_result = CMP_MATCH;
return ret;
}
static int ast_channel_by_exten_cb(void *obj, void *arg, void *data, int flags)
{
struct ast_channel *chan = obj;
struct ast_channel_callback_data *callback_data = data;
char *context = arg;
char *exten = data;
char *exten = (char *) callback_data->data_additional;
int ret = CMP_MATCH;
if (ast_strlen_zero(exten) || ast_strlen_zero(context)) {
@ -1315,7 +1378,12 @@ static int ast_channel_by_exten_cb(void *obj, void *arg, void *data, int flags)
return CMP_STOP;
}
ast_channel_lock(chan);
if (ast_channel_trylock(chan) != 0) {
/* Lock failure means the caller will need to retry until we get the lock we need */
callback_data->final_match_result = CMP_RETRY_NEEDED;
return CMP_STOP;
}
if (strcasecmp(ast_channel_context(chan), context) && strcasecmp(ast_channel_macrocontext(chan), context)) {
ret = 0; /* Context match failed, continue */
} else if (strcasecmp(ast_channel_exten(chan), exten) && strcasecmp(ast_channel_macroexten(chan), exten)) {
@ -1323,6 +1391,8 @@ static int ast_channel_by_exten_cb(void *obj, void *arg, void *data, int flags)
}
ast_channel_unlock(chan);
callback_data->final_match_result = ret;
return ret;
}
@ -1330,7 +1400,8 @@ static int ast_channel_by_uniqueid_cb(void *obj, void *arg, void *data, int flag
{
struct ast_channel *chan = obj;
char *uniqueid = arg;
size_t id_len = *(size_t *) data;
struct ast_channel_callback_data *callback_data = data;
size_t id_len = *(size_t *) callback_data->data_additional;
int ret = CMP_MATCH;
if (ast_strlen_zero(uniqueid)) {
@ -1338,13 +1409,20 @@ static int ast_channel_by_uniqueid_cb(void *obj, void *arg, void *data, int flag
return CMP_STOP;
}
ast_channel_lock(chan);
if (ast_channel_trylock(chan) != 0) {
/* Lock failure means the caller will need to retry until we get the lock we need */
callback_data->final_match_result = CMP_RETRY_NEEDED;
return CMP_STOP;
}
if ((!id_len && strcasecmp(ast_channel_uniqueid(chan), uniqueid))
|| (id_len && strncasecmp(ast_channel_uniqueid(chan), uniqueid, id_len))) {
ret = 0; /* uniqueid match failed, keep looking */
}
ast_channel_unlock(chan);
callback_data->final_match_result = CMP_MATCH;
return ret;
}
@ -1365,18 +1443,39 @@ struct ast_channel_iterator *ast_channel_iterator_destroy(struct ast_channel_ite
return NULL;
}
/*! \brief Internal helper for locating a channel by exten@context */
static struct ast_channel *channel_find_by_exten(const char *exten, const char *context, int ao2_flags)
{
char *l_context = (char *) context;
char *l_exten = (char *) exten;
struct ast_str *search_type_data = ast_str_create(64);
struct ast_channel *chan;
if (!search_type_data) {
ast_log(LOG_WARNING, "Memory allocation failure\n");
return NULL;
}
ast_str_set(&search_type_data, 0, "%s@%s", exten, context);
chan = ast_channel_callback_safe(ast_channel_by_exten_cb,
"extension@context", ast_str_buffer(search_type_data), l_context, l_exten, ao2_flags);
ast_free(search_type_data);
return chan;
}
struct ast_channel_iterator *ast_channel_iterator_by_exten_new(const char *exten, const char *context)
{
struct ast_channel_iterator *i;
char *l_exten = (char *) exten;
char *l_context = (char *) context;
if (!(i = ast_calloc(1, sizeof(*i)))) {
return NULL;
}
i->active_iterator = (void *) ast_channel_callback(ast_channel_by_exten_cb,
l_context, l_exten, OBJ_MULTIPLE);
i->active_iterator = (void *) channel_find_by_exten(exten, context, OBJ_MULTIPLE);
if (!i->active_iterator) {
ast_free(i);
return NULL;
@ -1394,9 +1493,10 @@ struct ast_channel_iterator *ast_channel_iterator_by_name_new(const char *name,
return NULL;
}
i->active_iterator = (void *) ast_channel_callback(ast_channel_by_name_cb,
l_name, &name_len,
i->active_iterator = (void *) ast_channel_callback_safe(ast_channel_by_name_cb,
"channel name", name, l_name, &name_len,
OBJ_MULTIPLE | (name_len == 0 /* match the whole word, so optimize */ ? OBJ_KEY : 0));
if (!i->active_iterator) {
ast_free(i);
return NULL;
@ -1431,6 +1531,7 @@ static int ast_channel_cmp_cb(void *obj, void *arg, int flags)
return CMP_STOP;
}
struct ast_channel *ast_channel_get_by_name_prefix(const char *name, size_t name_len)
{
struct ast_channel *chan;
@ -1441,14 +1542,18 @@ struct ast_channel *ast_channel_get_by_name_prefix(const char *name, size_t name
return NULL;
}
chan = ast_channel_callback(ast_channel_by_name_cb, l_name, &name_len,
chan = ast_channel_callback_safe(ast_channel_by_name_cb, "channel name", name, l_name, &name_len,
(name_len == 0) /* optimize if it is a complete name match */ ? OBJ_KEY : 0);
if (chan) {
return chan;
}
/* Now try a search for uniqueid. */
return ast_channel_callback(ast_channel_by_uniqueid_cb, l_name, &name_len, 0);
chan = ast_channel_callback_safe(ast_channel_by_uniqueid_cb, "channel uniqueid", name, l_name, &name_len, 0);
/* (chan != NULL) if we found a match */
return chan;
}
struct ast_channel *ast_channel_get_by_name(const char *name)
@ -1458,10 +1563,7 @@ struct ast_channel *ast_channel_get_by_name(const char *name)
struct ast_channel *ast_channel_get_by_exten(const char *exten, const char *context)
{
char *l_exten = (char *) exten;
char *l_context = (char *) context;
return ast_channel_callback(ast_channel_by_exten_cb, l_context, l_exten, 0);
return channel_find_by_exten(exten, context, 0);
}
int ast_is_deferrable_frame(const struct ast_frame *frame)

View File

@ -1484,9 +1484,14 @@ struct channel_set_debug_args {
static int channel_set_debug(void *obj, void *arg, void *data, int flags)
{
struct ast_channel *chan = obj;
struct channel_set_debug_args *args = data;
struct ast_channel_callback_data *callback_data = data;
struct channel_set_debug_args *args = callback_data->data_additional;
ast_channel_lock(chan);
if (ast_channel_trylock(chan) != 0) {
/* Lock failure means the caller will need to retry until we get the lock we need */
callback_data->final_match_result = CMP_RETRY_NEEDED;
return CMP_STOP;
}
if (!(ast_channel_fin(chan) & DEBUGCHAN_FLAG) || !(ast_channel_fout(chan) & DEBUGCHAN_FLAG)) {
if (args->is_off) {
@ -1556,7 +1561,7 @@ static char *handle_core_set_debug_channel(struct ast_cli_entry *e, int cmd, str
global_fin |= DEBUGCHAN_FLAG;
global_fout |= DEBUGCHAN_FLAG;
}
ast_channel_callback(channel_set_debug, NULL, &args, OBJ_NODATA | OBJ_MULTIPLE);
ast_channel_callback_safe(channel_set_debug, "all channels, where debug is not enabled", "all", NULL, &args, OBJ_NODATA | OBJ_MULTIPLE);
} else {
if ((c = ast_channel_get_by_name(a->argv[e->args]))) {
channel_set_debug(c, NULL, &args, 0);