From b35211fd58afcf430a0d95a243dc7a086d72b2b8 Mon Sep 17 00:00:00 2001 From: Jaco Kroon Date: Fri, 21 May 2021 20:11:59 +0200 Subject: [PATCH] Replacement patch for v13. Change-Id: I30236d7d7229f317c681fb7c6d7944d6108acd08 --- funcs/func_lock.c | 234 +++++++++++++++++++++++++--------------------- 1 file changed, 126 insertions(+), 108 deletions(-) diff --git a/funcs/func_lock.c b/funcs/func_lock.c index a006a574eb..c472504f52 100644 --- a/funcs/func_lock.c +++ b/funcs/func_lock.c @@ -44,6 +44,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") #include "asterisk/linkedlists.h" #include "asterisk/astobj2.h" #include "asterisk/utils.h" +#include "asterisk/cli.h" /*** DOCUMENTATION @@ -112,7 +113,6 @@ static AST_LIST_HEAD_STATIC(locklist, lock_frame); static void lock_free(void *data); static void lock_fixup(void *data, struct ast_channel *oldchan, struct ast_channel *newchan); static int unloading = 0; -static pthread_t broker_tid = AST_PTHREADT_NULL; static const struct ast_datastore_info lock_info = { .type = "MUTEX", @@ -126,8 +126,8 @@ struct lock_frame { ast_cond_t cond; /*! count is needed so if a recursive mutex exits early, we know how many times to unlock it. */ unsigned int count; - /*! Container of requesters for the named lock */ - struct ao2_container *requesters; + /*! Count of waiting of requesters for the named lock */ + unsigned int requesters; /*! who owns us */ struct ast_channel *owner; /*! name of the lock */ @@ -149,14 +149,19 @@ static void lock_free(void *data) while ((clframe = AST_LIST_REMOVE_HEAD(oldlist, list))) { /* Only unlock if we own the lock */ if (clframe->channel == clframe->lock_frame->owner) { + ast_mutex_lock(&clframe->lock_frame->mutex); clframe->lock_frame->count = 0; clframe->lock_frame->owner = NULL; + ast_cond_signal(&clframe->lock_frame->cond); + ast_mutex_unlock(&clframe->lock_frame->mutex); } ast_free(clframe); } AST_LIST_UNLOCK(oldlist); AST_LIST_HEAD_DESTROY(oldlist); ast_free(oldlist); + + ast_module_unref(ast_module_info->self); } static void lock_fixup(void *data, struct ast_channel *oldchan, struct ast_channel *newchan) @@ -175,54 +180,11 @@ static void lock_fixup(void *data, struct ast_channel *oldchan, struct ast_chann if (clframe->lock_frame->owner == oldchan) { clframe->lock_frame->owner = newchan; } - /* We don't move requesters, because the thread stack is different */ clframe->channel = newchan; } AST_LIST_UNLOCK(list); } -static void *lock_broker(void *unused) -{ - struct lock_frame *frame; - struct timespec forever = { 1000000, 0 }; - for (;;) { - int found_requester = 0; - - /* Test for cancel outside of the lock */ - pthread_testcancel(); - AST_LIST_LOCK(&locklist); - - AST_LIST_TRAVERSE(&locklist, frame, entries) { - if (ao2_container_count(frame->requesters)) { - found_requester++; - ast_mutex_lock(&frame->mutex); - if (!frame->owner) { - ast_cond_signal(&frame->cond); - } - ast_mutex_unlock(&frame->mutex); - } - } - - AST_LIST_UNLOCK(&locklist); - pthread_testcancel(); - - /* If there are no requesters, then wait for a signal */ - if (!found_requester) { - nanosleep(&forever, NULL); - } else { - sched_yield(); - } - } - /* Not reached */ - return NULL; -} - -static int ast_channel_cmp_cb(void *obj, void *arg, int flags) -{ - struct ast_channel *chan = obj, *cmp_args = arg; - return strcasecmp(ast_channel_name(chan), ast_channel_name(cmp_args)) ? 0 : CMP_MATCH; -} - static int get_lock(struct ast_channel *chan, char *lockname, int trylock) { struct ast_datastore *lock_store = ast_channel_datastore_find(chan, &lock_info, NULL); @@ -234,7 +196,12 @@ static int get_lock(struct ast_channel *chan, char *lockname, int trylock) struct timeval now; if (!lock_store) { - ast_debug(1, "Channel %s has no lock datastore, so we're allocating one.\n", ast_channel_name(chan)); + if (unloading) { + ast_log(LOG_ERROR, "%sLOCK has no datastore and func_lock is unloading, failing.\n", + trylock ? "TRY" : ""); + return -1; + } + lock_store = ast_datastore_alloc(&lock_info, NULL); if (!lock_store) { ast_log(LOG_ERROR, "Unable to allocate new datastore. No locks will be obtained.\n"); @@ -253,6 +220,9 @@ static int get_lock(struct ast_channel *chan, char *lockname, int trylock) lock_store->data = list; AST_LIST_HEAD_INIT(list); ast_channel_datastore_add(chan, lock_store); + + /* We cannot unload until this channel has released the lock_store */ + ast_module_ref(ast_module_info->self); } else list = lock_store->data; @@ -266,6 +236,9 @@ static int get_lock(struct ast_channel *chan, char *lockname, int trylock) if (!current) { if (unloading) { + ast_log(LOG_ERROR, + "Lock doesn't exist whilst unloading. %sLOCK will fail.\n", + trylock ? "TRY" : ""); /* Don't bother */ AST_LIST_UNLOCK(&locklist); return -1; @@ -292,17 +265,12 @@ static int get_lock(struct ast_channel *chan, char *lockname, int trylock) AST_LIST_UNLOCK(&locklist); return -1; } - current->requesters = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_MUTEX, 0, - NULL, ast_channel_cmp_cb); - if (!current->requesters) { - ast_mutex_destroy(¤t->mutex); - ast_cond_destroy(¤t->cond); - ast_free(current); - AST_LIST_UNLOCK(&locklist); - return -1; - } AST_LIST_INSERT_TAIL(&locklist, current, entries); } + /* Add to requester list */ + ast_mutex_lock(¤t->mutex); + current->requesters++; + ast_mutex_unlock(¤t->mutex); AST_LIST_UNLOCK(&locklist); /* Found lock or created one - now find or create the corresponding link in the channel */ @@ -315,7 +283,13 @@ static int get_lock(struct ast_channel *chan, char *lockname, int trylock) if (!clframe) { if (unloading) { + ast_log(LOG_ERROR, + "Busy unloading. %sLOCK will fail.\n", + trylock ? "TRY" : ""); /* Don't bother */ + ast_mutex_lock(¤t->mutex); + current->requesters--; + ast_mutex_unlock(¤t->mutex); AST_LIST_UNLOCK(list); return -1; } @@ -324,6 +298,9 @@ static int get_lock(struct ast_channel *chan, char *lockname, int trylock) ast_log(LOG_ERROR, "Unable to allocate channel lock frame. %sLOCK will fail.\n", trylock ? "TRY" : ""); + ast_mutex_lock(¤t->mutex); + current->requesters--; + ast_mutex_unlock(¤t->mutex); AST_LIST_UNLOCK(list); return -1; } @@ -339,44 +316,44 @@ static int get_lock(struct ast_channel *chan, char *lockname, int trylock) * the same amount, before we'll release this one. */ if (current->owner == chan) { + /* We're not a requester, we already have it */ + ast_mutex_lock(¤t->mutex); + current->requesters--; + ast_mutex_unlock(¤t->mutex); current->count++; return 0; } - /* Okay, we have both frames, so now we need to try to lock. - * - * Locking order: always lock locklist first. We need the - * locklist lock because the broker thread counts whether - * there are requesters with the locklist lock held, and we - * need to hold it, so that when we send our signal, below, - * to wake up the broker thread, it definitely will see that - * a requester exists at that point in time. Otherwise, we - * could add to the requesters after it has already seen that - * that lock is unoccupied and wait forever for another signal. - */ - AST_LIST_LOCK(&locklist); - ast_mutex_lock(¤t->mutex); - /* Add to requester list */ - ao2_link(current->requesters, chan); - pthread_kill(broker_tid, SIGURG); - AST_LIST_UNLOCK(&locklist); - /* Wait up to three seconds from now for LOCK. */ now = ast_tvnow(); timeout.tv_sec = now.tv_sec + 3; timeout.tv_nsec = now.tv_usec * 1000; - if (!current->owner - || (!trylock - && !(res = ast_cond_timedwait(¤t->cond, ¤t->mutex, &timeout)))) { - res = 0; + ast_mutex_lock(¤t->mutex); + + res = 0; + while (!trylock && !res && current->owner) { + res = ast_cond_timedwait(¤t->cond, ¤t->mutex, &timeout); + } + if (current->owner) { + ast_log(LOG_ERROR, "%sLOCK failed to obtain lock %s.\n", trylock ? "TRY" : "", + lockname); + /* timeout; + * trylock; or + * cond_timedwait failed. + * + * either way, we fail to obtain the lock. + */ + res = -1; + } else { current->owner = chan; current->count++; - } else { - res = -1; + res = 0; } /* Remove from requester list */ - ao2_unlink(current->requesters, chan); + current->requesters--; + if (res && unloading) + ast_cond_signal(¤t->cond); ast_mutex_unlock(¤t->mutex); return res; @@ -400,7 +377,7 @@ static int unlock_read(struct ast_channel *chan, const char *cmd, char *data, ch } if (!(list = lock_store->data)) { - ast_debug(1, "This should NEVER happen\n"); + ast_log(LOG_ERROR, "Datastore's data member is NULL ... this should be impossible."); ast_copy_string(buf, "0", len); return 0; } @@ -419,12 +396,17 @@ static int unlock_read(struct ast_channel *chan, const char *cmd, char *data, ch if (!clframe) { /* We didn't have this lock in the first place */ + ast_log(LOG_WARNING, "Attempting to UNLOCK(%s) - a lock this channel never owned.\n", + data); ast_copy_string(buf, "0", len); return 0; } if (--clframe->lock_frame->count == 0) { + ast_mutex_lock(&clframe->lock_frame->mutex); clframe->lock_frame->owner = NULL; + ast_cond_signal(&clframe->lock_frame->cond); + ast_mutex_unlock(&clframe->lock_frame->mutex); } ast_copy_string(buf, "1", len); @@ -455,6 +437,37 @@ static int trylock_read(struct ast_channel *chan, const char *cmd, char *data, c return 0; } +static char *handle_cli_locks_show(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a) +{ + int c = 0; + struct lock_frame* current; + switch (cmd) { + case CLI_INIT: + e->command = "locks show"; + e->usage = + "Usage: locks show\n" + " List all locks known to func_lock, along with their current status.\n"; + return NULL; + case CLI_GENERATE: + return NULL; + } + + ast_cli(a->fd, "func_lock locks:\n"); + ast_cli(a->fd, "%-40s Requesters Owner\n", "Name"); + AST_LIST_LOCK(&locklist); + AST_LIST_TRAVERSE(&locklist, current, entries) { + ast_mutex_lock(¤t->mutex); + ast_cli(a->fd, "%-40s %-10d %s\n", current->name, current->requesters, + current->owner ? ast_channel_name(current->owner) : "(unlocked)"); + ast_mutex_unlock(¤t->mutex); + c++; + } + AST_LIST_UNLOCK(&locklist); + ast_cli(a->fd, "%d total locks listed.\n", c); + + return 0; +} + static struct ast_custom_function lock_function = { .name = "LOCK", .read = lock_read, @@ -473,6 +486,8 @@ static struct ast_custom_function unlock_function = { .read_max = 2, }; +static struct ast_cli_entry cli_locks_show = AST_CLI_DEFINE(handle_cli_locks_show, "List func_lock locks."); + static int unload_module(void) { struct lock_frame *current; @@ -480,34 +495,43 @@ static int unload_module(void) /* Module flag */ unloading = 1; + /* Make it impossible for new requesters to be added + * NOTE: channels could already be in get_lock() */ + ast_custom_function_unregister(&lock_function); + ast_custom_function_unregister(&trylock_function); + + ast_cli_unregister(&cli_locks_show); + AST_LIST_LOCK(&locklist); while ((current = AST_LIST_REMOVE_HEAD(&locklist, entries))) { - /* If any locks are currently in use, then we cannot unload this module */ - if (current->owner || ao2_container_count(current->requesters)) { - /* Put it back */ - AST_LIST_INSERT_HEAD(&locklist, current, entries); - AST_LIST_UNLOCK(&locklist); - unloading = 0; - return -1; + int warned = 0; + ast_mutex_lock(¤t->mutex); + while (current->owner || current->requesters) { + if (!warned) { + ast_log(LOG_WARNING, "Waiting for %d requesters for %s lock %s.\n", + current->requesters, current->owner ? "locked" : "unlocked", + current->name); + warned = 1; + } + /* either the mutex is locked, or other parties are currently in get_lock, + * we need to wait for all of those to clear first */ + ast_cond_wait(¤t->cond, ¤t->mutex); } + ast_mutex_unlock(¤t->mutex); + /* At this point we know: + * 1. the lock has been released, + * 2. there are no requesters (nor should any be able to sneak in). + */ ast_mutex_destroy(¤t->mutex); - ao2_ref(current->requesters, -1); + ast_cond_destroy(¤t->cond); ast_free(current); } + AST_LIST_UNLOCK(&locklist); + AST_LIST_HEAD_DESTROY(&locklist); - /* No locks left, unregister functions */ - ast_custom_function_unregister(&lock_function); - ast_custom_function_unregister(&trylock_function); + /* At this point we can safely stop access to UNLOCK */ ast_custom_function_unregister(&unlock_function); - if (broker_tid != AST_PTHREADT_NULL) { - pthread_cancel(broker_tid); - pthread_kill(broker_tid, SIGURG); - pthread_join(broker_tid, NULL); - } - - AST_LIST_UNLOCK(&locklist); - return 0; } @@ -516,13 +540,7 @@ static int load_module(void) int res = ast_custom_function_register_escalating(&lock_function, AST_CFE_READ); res |= ast_custom_function_register_escalating(&trylock_function, AST_CFE_READ); res |= ast_custom_function_register_escalating(&unlock_function, AST_CFE_READ); - - if (ast_pthread_create_background(&broker_tid, NULL, lock_broker, NULL)) { - ast_log(LOG_ERROR, "Failed to start lock broker thread. Unloading func_lock module.\n"); - broker_tid = AST_PTHREADT_NULL; - unload_module(); - return AST_MODULE_LOAD_DECLINE; - } + res |= ast_cli_register(&cli_locks_show); return res; } -- 2.26.3