astobj2: Run weakproxy callbacks outside of lock.
authorCorey Farrell <git@cfware.com>
Tue, 10 Oct 2017 20:09:14 +0000 (16:09 -0400)
committerCorey Farrell <git@cfware.com>
Tue, 10 Oct 2017 22:23:00 +0000 (18:23 -0400)
Copy the list of weakproxy callbacks to temporary memory so they can be
run without holding the weakproxy lock.

Change-Id: Ib167622a8a0f873fd73938f7611b2a5914308047

include/asterisk/astobj2.h
main/astobj2.c

index 484e1e3..9b5ec12 100644 (file)
@@ -671,6 +671,10 @@ int ao2_weakproxy_subscribe(void *weakproxy, ao2_weakproxy_notification_cb cb, v
  *       of the cb / data pair.  If it was subscribed multiple times it must be
  *       unsubscribed as many times.  The OBJ_MULTIPLE flag can be used to remove
  *       matching subscriptions.
+ *
+ * \note When it's time to run callbacks they are copied to a temporary list so the
+ *       weakproxy can be unlocked before running.  That means it's possible for
+ *       this function to find nothing before the callback is run in another thread.
  */
 int ao2_weakproxy_unsubscribe(void *weakproxy, ao2_weakproxy_notification_cb cb, void *data, int flags);
 
index d534900..da2d44e 100644 (file)
@@ -82,8 +82,6 @@ struct ao2_weakproxy_notification {
        AST_LIST_ENTRY(ao2_weakproxy_notification) list;
 };
 
-static void weakproxy_run_callbacks(struct ao2_weakproxy *weakproxy);
-
 struct ao2_lock_priv {
        ast_mutex_t lock;
 };
@@ -469,7 +467,7 @@ int __ao2_ref(void *user_data, int delta,
        struct astobj2_lockobj *obj_lockobj;
        int current_value;
        int ret;
-       void *weakproxy = NULL;
+       struct ao2_weakproxy *weakproxy = NULL;
 
        if (obj == NULL) {
                if (ref_log && user_data) {
@@ -498,6 +496,8 @@ int __ao2_ref(void *user_data, int delta,
 #endif
 
        if (weakproxy) {
+               struct ao2_weakproxy cbs;
+
                if (current_value == 1) {
                        /* The only remaining reference is the one owned by the weak object */
                        struct astobj2 *internal_weakproxy;
@@ -508,8 +508,9 @@ int __ao2_ref(void *user_data, int delta,
                        internal_weakproxy->priv_data.weakptr = NULL;
                        obj->priv_data.weakptr = NULL;
 
-                       /* Notify the subscribers that weakproxy now points to NULL. */
-                       weakproxy_run_callbacks(weakproxy);
+                       /* transfer list to local copy so callbacks are run with weakproxy unlocked. */
+                       cbs.destroyed_cb = weakproxy->destroyed_cb;
+                       AST_LIST_HEAD_INIT_NOLOCK(&weakproxy->destroyed_cb);
 
                        /* weak is already unlinked from obj so this won't recurse */
                        ao2_ref(user_data, -1);
@@ -518,6 +519,14 @@ int __ao2_ref(void *user_data, int delta,
                ao2_unlock(weakproxy);
 
                if (current_value == 1) {
+                       struct ao2_weakproxy_notification *destroyed_cb;
+
+                       /* Notify the subscribers that weakproxy now points to NULL. */
+                       while ((destroyed_cb = AST_LIST_REMOVE_HEAD(&cbs.destroyed_cb, list))) {
+                               destroyed_cb->cb(weakproxy, destroyed_cb->data);
+                               ast_free(destroyed_cb);
+                       }
+
                        ao2_ref(weakproxy, -1);
                }
        }
@@ -796,16 +805,6 @@ void *__ao2_global_obj_ref(struct ao2_global_obj *holder, const char *tag, const
 }
 
 
-static void weakproxy_run_callbacks(struct ao2_weakproxy *weakproxy)
-{
-       struct ao2_weakproxy_notification *destroyed_cb;
-
-       while ((destroyed_cb = AST_LIST_REMOVE_HEAD(&weakproxy->destroyed_cb, list))) {
-               destroyed_cb->cb(weakproxy, destroyed_cb->data);
-               ast_free(destroyed_cb);
-       }
-}
-
 void *__ao2_weakproxy_alloc(size_t data_size, ao2_destructor_fn destructor_fn,
        const char *tag, const char *file, int line, const char *func)
 {
@@ -951,6 +950,7 @@ int ao2_weakproxy_subscribe(void *weakproxy, ao2_weakproxy_notification_cb cb, v
 {
        struct astobj2 *weakproxy_internal = INTERNAL_OBJ_CHECK(weakproxy);
        int ret = -1;
+       int hasobj;
 
        if (!weakproxy_internal || weakproxy_internal->priv_data.magic != AO2_WEAK) {
                return -1;
@@ -960,7 +960,8 @@ int ao2_weakproxy_subscribe(void *weakproxy, ao2_weakproxy_notification_cb cb, v
                ao2_lock(weakproxy);
        }
 
-       if (weakproxy_internal->priv_data.weakptr) {
+       hasobj = weakproxy_internal->priv_data.weakptr != NULL;
+       if (hasobj) {
                struct ao2_weakproxy *weak = weakproxy;
                struct ao2_weakproxy_notification *sub = ast_calloc(1, sizeof(*sub));
 
@@ -970,15 +971,17 @@ int ao2_weakproxy_subscribe(void *weakproxy, ao2_weakproxy_notification_cb cb, v
                        AST_LIST_INSERT_HEAD(&weak->destroyed_cb, sub, list);
                        ret = 0;
                }
-       } else {
-               cb(weakproxy, data);
-               ret = 0;
        }
 
        if (!(flags & OBJ_NOLOCK)) {
                ao2_unlock(weakproxy);
        }
 
+       if (!hasobj) {
+               cb(weakproxy, data);
+               ret = 0;
+       }
+
        return ret;
 }