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

diff --git a/include/asterisk/sorcery.h b/include/asterisk/sorcery.h
index 896633816839e59bd5bfe1cff438d7ff660ba507..bfb2c39ad3415567e69253cb901b8f88e348ea8b 100644 (file)
--- 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 0bb2826c818854b030605cb0c47a59e60ad3b0e1..6694167cd23365f015c9959f4d7fa730a060d7b2 100644 (file)
--- 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;