From b8dadccbe148c9113565f497da39a5e97d4ac533 Mon Sep 17 00:00:00 2001 From: Corey Farrell Date: Mon, 9 Oct 2017 22:00:45 -0400 Subject: [PATCH] sorcery: Use ao2_weakproxy to hold list of instances. * Store weak proxy objects in instances container. * Remove special unreference function and replace with macro that calls ao2_cleanup. * Add REF_DEBUG information to ast_sorcery_open. Change-Id: I5a150a4e13cee319d46b5a4654f95a4623a978f8 --- include/asterisk/sorcery.h | 11 +++-- main/sorcery.c | 95 +++++++++++++++++++++++--------------- 2 files changed, 67 insertions(+), 39 deletions(-) diff --git a/include/asterisk/sorcery.h b/include/asterisk/sorcery.h index 8966338168..bfb2c39ad3 100644 --- a/include/asterisk/sorcery.h +++ b/include/asterisk/sorcery.h @@ -392,9 +392,9 @@ int ast_sorcery_wizard_unregister(const struct ast_sorcery_wizard *interface); * \retval non-NULL success * \retval NULL if allocation failed */ -struct ast_sorcery *__ast_sorcery_open(const char *module); +struct ast_sorcery *__ast_sorcery_open(const char *module, const char *file, int line, const char *func); -#define ast_sorcery_open() __ast_sorcery_open(AST_MODULE) +#define ast_sorcery_open() __ast_sorcery_open(AST_MODULE, __FILE__, __LINE__, __PRETTY_FUNCTION__) /*! * \brief Retrieves an existing sorcery instance by module name @@ -1280,8 +1280,13 @@ int ast_sorcery_is_stale(const struct ast_sorcery *sorcery, void *object); * \brief Decrease the reference count of a sorcery structure * * \param sorcery Pointer to a sorcery structure + * + * \note Prior to 16.0.0 this was a function which had to be used. + * Now you can use any variant of ao2_cleanup or ao2_ref to + * release a reference. */ -void ast_sorcery_unref(struct ast_sorcery *sorcery); +#define ast_sorcery_unref(sorcery) \ + ao2_cleanup(sorcery) /*! * \brief Get the unique identifier of a sorcery object diff --git a/main/sorcery.c b/main/sorcery.c index 0bb2826c81..6694167cd2 100644 --- a/main/sorcery.c +++ b/main/sorcery.c @@ -207,6 +207,13 @@ struct ast_sorcery_object_field { intptr_t args[]; }; +/*! \brief Proxy object for sorcery */ +struct sorcery_proxy { + AO2_WEAKPROXY(); + /*! \brief The name of the module owning this sorcery instance */ + char module_name[0]; +}; + /*! \brief Full structure for sorcery */ struct ast_sorcery { /*! \brief Container for known object types */ @@ -215,8 +222,8 @@ struct ast_sorcery { /*! \brief Observers */ struct ao2_container *observers; - /*! \brief The name of the module owning this sorcery instance */ - char module_name[0]; + /*! \brief Pointer to module_name in the associated sorcery_proxy. */ + char *module_name; }; /*! \brief Structure for passing load/reload details */ @@ -449,8 +456,8 @@ static void sorcery_cleanup(void) /*! \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 struct sorcery_proxy *object_left = obj; + const struct sorcery_proxy *object_right = arg; const char *right_key = arg; int cmp; @@ -477,7 +484,7 @@ static int sorcery_instance_cmp(void *obj, void *arg, int flags) /*! \brief Hashing function for sorcery instances */ static int sorcery_instance_hash(const void *obj, const int flags) { - const struct ast_sorcery *object; + const struct sorcery_proxy *object; const char *key; switch (flags & OBJ_SEARCH_MASK) { @@ -752,55 +759,83 @@ static int sorcery_type_cmp(void *obj, void *arg, int flags) return CMP_MATCH; } -struct ast_sorcery *__ast_sorcery_open(const char *module_name) +static void sorcery_proxy_cb(void *weakproxy, void *data) { + ao2_unlink(instances, weakproxy); +} + +struct ast_sorcery *__ast_sorcery_open(const char *module_name, const char *file, int line, const char *func) +{ + struct sorcery_proxy *proxy; struct ast_sorcery *sorcery; ast_assert(module_name != NULL); ao2_wrlock(instances); - if ((sorcery = ao2_find(instances, module_name, OBJ_SEARCH_KEY | OBJ_NOLOCK))) { - goto done; + sorcery = __ao2_weakproxy_find(instances, module_name, OBJ_SEARCH_KEY | OBJ_NOLOCK, + __PRETTY_FUNCTION__, file, line, func); + if (sorcery) { + ao2_unlock(instances); + + return sorcery; } - if (!(sorcery = ao2_alloc(sizeof(*sorcery) + strlen(module_name) + 1, sorcery_destructor))) { - goto done; + proxy = ao2_t_weakproxy_alloc(sizeof(*proxy) + strlen(module_name) + 1, NULL, module_name); + if (!proxy) { + goto failure_cleanup; + } + strcpy(proxy->module_name, module_name); /* Safe */ + + sorcery = __ao2_alloc(sizeof(*sorcery), sorcery_destructor, AO2_ALLOC_OPT_LOCK_MUTEX, module_name, file, line, func); + if (!sorcery) { + goto failure_cleanup; + } + + sorcery->module_name = proxy->module_name; + + /* We have exclusive access to proxy and sorcery, no need for locking here. */ + if (ao2_t_weakproxy_set_object(proxy, sorcery, OBJ_NOLOCK, "weakproxy link")) { + goto failure_cleanup; + } + + if (ao2_weakproxy_subscribe(proxy, sorcery_proxy_cb, NULL, OBJ_NOLOCK)) { + goto failure_cleanup; } 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; + goto failure_cleanup; } if (!(sorcery->observers = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_RWLOCK, 0, NULL, NULL))) { - ao2_ref(sorcery, -1); - sorcery = NULL; - goto done; + goto failure_cleanup; } - strcpy(sorcery->module_name, module_name); /* Safe */ - if (__ast_sorcery_apply_config(sorcery, module_name, module_name) == AST_SORCERY_APPLY_FAIL) { ast_log(LOG_ERROR, "Error attempting to apply configuration %s to sorcery.\n", module_name); - ao2_cleanup(sorcery); - sorcery = NULL; - goto done; + goto failure_cleanup; } - ao2_link_flags(instances, sorcery, OBJ_NOLOCK); + ao2_link_flags(instances, proxy, OBJ_NOLOCK); + ao2_ref(proxy, -1); NOTIFY_GLOBAL_OBSERVERS(observers, instance_created, module_name, sorcery); -done: ao2_unlock(instances); return sorcery; + +failure_cleanup: + /* cleanup of sorcery may result in locking instances, so make sure we unlock first. */ + ao2_unlock(instances); + ao2_cleanup(sorcery); + ao2_cleanup(proxy); + + return NULL; } /*! \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); + return ao2_weakproxy_find(instances, module_name, OBJ_SEARCH_KEY, ""); } /*! \brief Destructor function for object types */ @@ -2293,18 +2328,6 @@ int ast_sorcery_is_stale(const struct ast_sorcery *sorcery, void *object) return res; } -void ast_sorcery_unref(struct ast_sorcery *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) { const struct ast_sorcery_object_details *details = object; -- 2.47.2