From a94c8562fd70bc5b317a434db36f19fd34c70f8c Mon Sep 17 00:00:00 2001 From: George Joseph <george.joseph@fairview5.com> Date: Thu, 20 Feb 2014 20:45:30 +0000 Subject: [PATCH] sorcery: Create sorcery instance registry. In order to retrieve an arbitrary sorcery instance from a dialplan function (or any place else) there needs to be a registry of sorcery instances. ast_sorcery_init now creates a hashtab as a registry. ast_sorcery_open now checks the hashtab for an existing sorcery instance matching the caller's module name. If it finds one, it bumps the refcount and returns it. If not, it creates a new sorcery instance, adds it to the hashtab, then returns it. ast_sorcery_retrieve_by_module_name is a new function that does a hashtab lookup by module name. It can be called by the future dialplan function. res_pjsip/config_system needed a small change to share the main res_pjsip sorcery instance. tests/test_sorcery was updated to include a test for the registry. (closes issue ASTERISK-22537) Review: http://reviewboard.asterisk.org/r/3184/ ........ Merged revisions 408518 from http://svn.asterisk.org/svn/asterisk/branches/12 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@408519 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- include/asterisk/sorcery.h | 20 ++- main/sorcery.c | 194 +++++++++++++++++++--- res/res_pjsip.c | 6 + res/res_pjsip/config_system.c | 10 +- res/res_pjsip/include/res_pjsip_private.h | 5 + tests/test_sorcery.c | 55 +++++- 6 files changed, 267 insertions(+), 23 deletions(-) diff --git a/include/asterisk/sorcery.h b/include/asterisk/sorcery.h index 47ad5e40f7..cf11528d1f 100644 --- a/include/asterisk/sorcery.h +++ b/include/asterisk/sorcery.h @@ -302,10 +302,28 @@ int ast_sorcery_wizard_unregister(const struct ast_sorcery_wizard *interface); /*! * \brief Open a new sorcery structure * + * \param module The module name (AST_MODULE) + * * \retval non-NULL success * \retval NULL if allocation failed */ -struct ast_sorcery *ast_sorcery_open(void); +struct ast_sorcery *__ast_sorcery_open(const char *module); + +#define ast_sorcery_open() __ast_sorcery_open(AST_MODULE) + +/*! + * \brief Retrieves an existing sorcery instance by module name + * + * \param module The module name + * + * \retval non-NULL success + * \retval NULL if no instance was found + * + * \note The returned instance has its reference count incremented. The caller + * must decrement the count when they're finished with it. + * + */ +struct ast_sorcery *ast_sorcery_retrieve_by_module_name(const char *module); /*! * \brief Apply configured wizard mappings diff --git a/main/sorcery.c b/main/sorcery.c index b3620ddf05..5104e3ada9 100644 --- a/main/sorcery.c +++ b/main/sorcery.c @@ -55,6 +55,9 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") /*! \brief Number of buckets for types (should be prime for performance reasons) */ #define TYPE_BUCKETS 53 +/*! \brief Number of buckets for instances (should be prime for performance reasons) */ +#define INSTANCE_BUCKETS 17 + /*! \brief Thread pool for observers */ static struct ast_threadpool *threadpool; @@ -161,6 +164,8 @@ struct ast_sorcery_object_wizard { struct ast_sorcery { /*! \brief Container for known object types */ struct ao2_container *types; + /*! \brief The name of the module owning this sorcery instance */ + char module_name[0]; }; /*! \brief Structure for passing load/reload details */ @@ -176,7 +181,10 @@ struct sorcery_load_details { }; /*! \brief Registered sorcery wizards */ -struct ao2_container *wizards; +static struct ao2_container *wizards; + +/*! \brief Registered sorcery instances */ +static struct ao2_container *instances; static int int_handler_fn(const void *obj, const intptr_t *args, char **buf) { @@ -250,19 +258,50 @@ static sorcery_field_handler sorcery_field_default_handler(enum aco_option_type /*! \brief Hashing function for sorcery wizards */ static int sorcery_wizard_hash(const void *obj, const int flags) { - const struct ast_sorcery_wizard *wizard = obj; - const char *name = obj; + const struct ast_sorcery_wizard *object; + const char *key; - return ast_str_hash(flags & OBJ_KEY ? name : wizard->name); + switch (flags & OBJ_SEARCH_MASK) { + case OBJ_SEARCH_KEY: + key = obj; + break; + case OBJ_SEARCH_OBJECT: + object = obj; + key = object->name; + break; + default: + ast_assert(0); + return 0; + } + return ast_str_hash(key); } /*! \brief Comparator function for sorcery wizards */ static int sorcery_wizard_cmp(void *obj, void *arg, int flags) { - struct ast_sorcery_wizard *wizard1 = obj, *wizard2 = arg; - const char *name = arg; - - return !strcmp(wizard1->name, flags & OBJ_KEY ? name : wizard2->name) ? CMP_MATCH | CMP_STOP : 0; + const struct ast_sorcery_wizard *object_left = obj; + const struct ast_sorcery_wizard *object_right = arg; + const char *right_key = arg; + int cmp; + + switch (flags & OBJ_SEARCH_MASK) { + case OBJ_SEARCH_OBJECT: + right_key = object_right->name; + /* Fall through */ + case OBJ_SEARCH_KEY: + cmp = strcmp(object_left->name, right_key); + break; + case OBJ_SEARCH_PARTIAL_KEY: + cmp = strncmp(object_left->name, right_key, strlen(right_key)); + break; + default: + cmp = 0; + break; + } + if (cmp) { + return 0; + } + return CMP_MATCH; } /*! \brief Cleanup function */ @@ -276,6 +315,58 @@ static void sorcery_exit(void) static void sorcery_cleanup(void) { ao2_cleanup(wizards); + wizards = NULL; + ao2_cleanup(instances); + instances = NULL; +} + +/*! \brief Compare function for sorcery instances */ +static int sorcery_instance_cmp(void *obj, void *arg, int flags) +{ + const struct ast_sorcery *object_left = obj; + const struct ast_sorcery *object_right = arg; + const char *right_key = arg; + int cmp; + + switch (flags & OBJ_SEARCH_MASK) { + case OBJ_SEARCH_OBJECT: + right_key = object_right->module_name; + /* Fall through */ + case OBJ_SEARCH_KEY: + cmp = strcmp(object_left->module_name, right_key); + break; + case OBJ_SEARCH_PARTIAL_KEY: + cmp = strncmp(object_left->module_name, right_key, strlen(right_key)); + break; + default: + cmp = 0; + break; + } + if (cmp) { + return 0; + } + return CMP_MATCH; +} + +/*! \brief Hashing function for sorcery instances */ +static int sorcery_instance_hash(const void *obj, const int flags) +{ + const struct ast_sorcery *object; + const char *key; + + switch (flags & OBJ_SEARCH_MASK) { + case OBJ_SEARCH_KEY: + key = obj; + break; + case OBJ_SEARCH_OBJECT: + object = obj; + key = object->module_name; + break; + default: + ast_assert(0); + return 0; + } + return ast_str_hash(key); } int ast_sorcery_init(void) @@ -299,6 +390,14 @@ int ast_sorcery_init(void) return -1; } + instances = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_RWLOCK, INSTANCE_BUCKETS, + sorcery_instance_hash, sorcery_instance_cmp); + if (!instances) { + sorcery_cleanup(); + sorcery_exit(); + return -1; + } + ast_register_cleanup(sorcery_cleanup); ast_register_atexit(sorcery_exit); @@ -362,37 +461,87 @@ static void sorcery_destructor(void *obj) /*! \brief Hashing function for sorcery types */ static int sorcery_type_hash(const void *obj, const int flags) { - const struct ast_sorcery_object_type *type = obj; - const char *name = obj; + const struct ast_sorcery_object_type *object; + const char *key; - return ast_str_hash(flags & OBJ_KEY ? name : type->name); + switch (flags & OBJ_SEARCH_MASK) { + case OBJ_SEARCH_KEY: + key = obj; + break; + case OBJ_SEARCH_OBJECT: + object = obj; + key = object->name; + break; + default: + ast_assert(0); + return 0; + } + return ast_str_hash(key); } /*! \brief Comparator function for sorcery types */ static int sorcery_type_cmp(void *obj, void *arg, int flags) { - struct ast_sorcery_object_type *type1 = obj, *type2 = arg; - const char *name = arg; - - return !strcmp(type1->name, flags & OBJ_KEY ? name : type2->name) ? CMP_MATCH | CMP_STOP : 0; + const struct ast_sorcery_object_type *object_left = obj; + const struct ast_sorcery_object_type *object_right = arg; + const char *right_key = arg; + int cmp; + + switch (flags & OBJ_SEARCH_MASK) { + case OBJ_SEARCH_OBJECT: + right_key = object_right->name; + /* Fall through */ + case OBJ_SEARCH_KEY: + cmp = strcmp(object_left->name, right_key); + break; + case OBJ_SEARCH_PARTIAL_KEY: + cmp = strncmp(object_left->name, right_key, strlen(right_key)); + break; + default: + cmp = 0; + break; + } + if (cmp) { + return 0; + } + return CMP_MATCH; } -struct ast_sorcery *ast_sorcery_open(void) +struct ast_sorcery *__ast_sorcery_open(const char *module_name) { struct ast_sorcery *sorcery; - if (!(sorcery = ao2_alloc(sizeof(*sorcery), sorcery_destructor))) { - return NULL; + ast_assert(module_name != NULL); + + ao2_wrlock(instances); + if ((sorcery = ao2_find(instances, module_name, OBJ_SEARCH_KEY | OBJ_NOLOCK))) { + goto done; + } + + if (!(sorcery = ao2_alloc(sizeof(*sorcery) + strlen(module_name) + 1, sorcery_destructor))) { + goto done; } if (!(sorcery->types = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_RWLOCK, TYPE_BUCKETS, sorcery_type_hash, sorcery_type_cmp))) { ao2_ref(sorcery, -1); sorcery = NULL; + goto done; } + strcpy(sorcery->module_name, module_name); /* Safe */ + ao2_link_flags(instances, sorcery, OBJ_NOLOCK); + +done: + ao2_unlock(instances); return sorcery; } +/*! \brief Search function for sorcery instances */ +struct ast_sorcery *ast_sorcery_retrieve_by_module_name(const char *module_name) +{ + return ao2_find(instances, module_name, OBJ_SEARCH_KEY); +} + /*! \brief Destructor function for object types */ static void sorcery_object_type_destructor(void *obj) { @@ -1480,7 +1629,14 @@ int ast_sorcery_delete(const struct ast_sorcery *sorcery, void *object) void ast_sorcery_unref(struct ast_sorcery *sorcery) { - ao2_cleanup(sorcery); + if (sorcery) { + /* One ref for what we just released, the other for the instances container. */ + ao2_wrlock(instances); + if (ao2_ref(sorcery, -1) == 2) { + ao2_unlink_flags(instances, sorcery, OBJ_NOLOCK); + } + ao2_unlock(instances); + } } const char *ast_sorcery_object_get_id(const void *object) diff --git a/res/res_pjsip.c b/res/res_pjsip.c index 8600c8eb51..ef6c5b2734 100644 --- a/res/res_pjsip.c +++ b/res/res_pjsip.c @@ -2296,6 +2296,7 @@ static int load_module(void) sip_threadpool = ast_threadpool_create("SIP", NULL, &options); if (!sip_threadpool) { ast_log(LOG_ERROR, "Failed to create SIP threadpool. Aborting load\n"); + ast_sip_destroy_system(); pj_pool_release(memory_pool); memory_pool = NULL; pjsip_endpt_destroy(ast_pjsip_endpoint); @@ -2312,6 +2313,7 @@ static int load_module(void) NULL, PJ_THREAD_DEFAULT_STACK_SIZE * 2, 0, &monitor_thread); if (status != PJ_SUCCESS) { ast_log(LOG_ERROR, "Failed to start SIP monitor thread. Aborting load\n"); + ast_sip_destroy_system(); pj_pool_release(memory_pool); memory_pool = NULL; pjsip_endpt_destroy(ast_pjsip_endpoint); @@ -2326,6 +2328,7 @@ static int load_module(void) ast_log(LOG_ERROR, "Failed to initialize SIP configuration. Aborting load\n"); ast_sip_destroy_global_headers(); stop_monitor_thread(); + ast_sip_destroy_system(); pj_pool_release(memory_pool); memory_pool = NULL; pjsip_endpt_destroy(ast_pjsip_endpoint); @@ -2339,6 +2342,7 @@ static int load_module(void) ast_res_pjsip_destroy_configuration(); ast_sip_destroy_global_headers(); stop_monitor_thread(); + ast_sip_destroy_system(); pj_pool_release(memory_pool); memory_pool = NULL; pjsip_endpt_destroy(ast_pjsip_endpoint); @@ -2353,6 +2357,7 @@ static int load_module(void) ast_res_pjsip_destroy_configuration(); ast_sip_destroy_global_headers(); stop_monitor_thread(); + ast_sip_destroy_system(); pj_pool_release(memory_pool); memory_pool = NULL; pjsip_endpt_destroy(ast_pjsip_endpoint); @@ -2368,6 +2373,7 @@ static int load_module(void) ast_res_pjsip_destroy_configuration(); ast_sip_destroy_global_headers(); stop_monitor_thread(); + ast_sip_destroy_system(); pj_pool_release(memory_pool); memory_pool = NULL; pjsip_endpt_destroy(ast_pjsip_endpoint); diff --git a/res/res_pjsip/config_system.c b/res/res_pjsip/config_system.c index 5303ddfaee..bdf53149f4 100644 --- a/res/res_pjsip/config_system.c +++ b/res/res_pjsip/config_system.c @@ -120,7 +120,7 @@ int ast_sip_initialize_system(void) ast_sorcery_apply_default(system_sorcery, "system", "config", "pjsip.conf,criteria=type=system"); - if (ast_sorcery_object_register(system_sorcery, "system", system_alloc, NULL, system_apply)) { + if (ast_sorcery_object_register_no_reload(system_sorcery, "system", system_alloc, NULL, system_apply)) { ast_log(LOG_ERROR, "Failed to register with sorcery (is res_sorcery_config loaded?)\n"); ast_sorcery_unref(system_sorcery); system_sorcery = NULL; @@ -156,13 +156,21 @@ int ast_sip_initialize_system(void) system = ast_sorcery_alloc(system_sorcery, "system", NULL); if (!system) { ast_log(LOG_ERROR, "Unable to allocate default system config.\n"); + ast_sorcery_unref(system_sorcery); return -1; } if (system_apply(system_sorcery, system)) { ast_log(LOG_ERROR, "Failed to apply default system config.\n"); + ast_sorcery_unref(system_sorcery); return -1; } return 0; } + +void ast_sip_destroy_system(void) +{ + ast_sorcery_unref(system_sorcery); +} + diff --git a/res/res_pjsip/include/res_pjsip_private.h b/res/res_pjsip/include/res_pjsip_private.h index 87ff7f3ace..00aeea49fb 100644 --- a/res/res_pjsip/include/res_pjsip_private.h +++ b/res/res_pjsip/include/res_pjsip_private.h @@ -67,6 +67,11 @@ int ast_sip_initialize_outbound_authentication(void); */ int ast_sip_initialize_system(void); +/*! + * \brief Destroy system configuration + */ +void ast_sip_destroy_system(void); + /*! * \brief Initialize global configuration * diff --git a/tests/test_sorcery.c b/tests/test_sorcery.c index d2cd97c78f..fe1992bc09 100644 --- a/tests/test_sorcery.c +++ b/tests/test_sorcery.c @@ -35,6 +35,7 @@ ASTERISK_FILE_VERSION(__FILE__, "") #include "asterisk/test.h" #include "asterisk/module.h" +#include "asterisk/astobj2.h" #include "asterisk/sorcery.h" #include "asterisk/logger.h" #include "asterisk/json.h" @@ -294,24 +295,74 @@ AST_TEST_DEFINE(wizard_registration) AST_TEST_DEFINE(sorcery_open) { RAII_VAR(struct ast_sorcery *, sorcery, NULL, ast_sorcery_unref); + RAII_VAR(struct ast_sorcery *, sorcery2, NULL, ast_sorcery_unref); + int refcount; switch (cmd) { case TEST_INIT: info->name = "open"; info->category = "/main/sorcery/"; - info->summary = "sorcery open unit test"; + info->summary = "sorcery open/close unit test"; info->description = - "Test opening of sorcery"; + "Test opening of sorcery and registry operations"; return AST_TEST_NOT_RUN; case TEST_EXECUTE: break; } + if ((sorcery = ast_sorcery_retrieve_by_module_name(AST_MODULE))) { + ast_test_status_update(test, "There should NOT have been an existing sorcery instance\n"); + return AST_TEST_FAIL; + } + if (!(sorcery = ast_sorcery_open())) { ast_test_status_update(test, "Failed to open new sorcery structure\n"); return AST_TEST_FAIL; } + if (!(sorcery2 = ast_sorcery_retrieve_by_module_name(AST_MODULE))) { + ast_test_status_update(test, "Failed to find sorcery structure in registry\n"); + return AST_TEST_FAIL; + } + + if (sorcery2 != sorcery) { + ast_test_status_update(test, "Should have gotten same sorcery on retrieve\n"); + return AST_TEST_FAIL; + } + ast_sorcery_unref(sorcery2); + + if ((refcount = ao2_ref(sorcery, 0)) != 2) { + ast_test_status_update(test, "Should have been 2 references to sorcery instead of %d\n", refcount); + return AST_TEST_FAIL; + } + + if (!(sorcery2 = ast_sorcery_open())) { + ast_test_status_update(test, "Failed to open second sorcery structure\n"); + return AST_TEST_FAIL; + } + + if (sorcery2 != sorcery) { + ast_test_status_update(test, "Should have gotten same sorcery on 2nd open\n"); + return AST_TEST_FAIL; + } + + if ((refcount = ao2_ref(sorcery, 0)) != 3) { + ast_test_status_update(test, "Should have been 3 references to sorcery instead of %d\n", refcount); + return AST_TEST_FAIL; + } + + ast_sorcery_unref(sorcery); + ast_sorcery_unref(sorcery2); + + sorcery2 = NULL; + + if ((sorcery = ast_sorcery_retrieve_by_module_name(AST_MODULE))) { + ast_sorcery_unref(sorcery); + sorcery = NULL; + ast_test_status_update(test, "Should NOT have found sorcery structure in registry\n"); + return AST_TEST_FAIL; + } + return AST_TEST_PASS; } -- GitLab