<p>Corey Farrell has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6733">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">astobj2: Run weakproxy callbacks outside of lock.<br><br>Copy the list of weakproxy callbacks to temporary memory so they can be<br>run without holding the weakproxy lock.<br><br>Change-Id: Ib167622a8a0f873fd73938f7611b2a5914308047<br>---<br>M include/asterisk/astobj2.h<br>M main/astobj2.c<br>2 files changed, 18 insertions(+), 15 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/33/6733/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/astobj2.h b/include/asterisk/astobj2.h<br>index 484e1e3..9b5ec12 100644<br>--- a/include/asterisk/astobj2.h<br>+++ b/include/asterisk/astobj2.h<br>@@ -671,6 +671,10 @@<br>  *       of the cb / data pair.  If it was subscribed multiple times it must be<br>  *       unsubscribed as many times.  The OBJ_MULTIPLE flag can be used to remove<br>  *       matching subscriptions.<br>+ *<br>+ * \note When it's time to run callbacks they are copied to a temporary list so the<br>+ *       weakproxy can be unlocked before running.  That means it's possible for<br>+ *       this function to find nothing before the callback is run in another thread.<br>  */<br> int ao2_weakproxy_unsubscribe(void *weakproxy, ao2_weakproxy_notification_cb cb, void *data, int flags);<br> <br>diff --git a/main/astobj2.c b/main/astobj2.c<br>index d534900..8162b25 100644<br>--- a/main/astobj2.c<br>+++ b/main/astobj2.c<br>@@ -82,8 +82,6 @@<br>  AST_LIST_ENTRY(ao2_weakproxy_notification) list;<br> };<br> <br>-static void weakproxy_run_callbacks(struct ao2_weakproxy *weakproxy);<br>-<br> struct ao2_lock_priv {<br>  ast_mutex_t lock;<br> };<br>@@ -469,7 +467,7 @@<br>   struct astobj2_lockobj *obj_lockobj;<br>  int current_value;<br>    int ret;<br>-     void *weakproxy = NULL;<br>+      struct ao2_weakproxy *weakproxy = NULL;<br> <br>    if (obj == NULL) {<br>            if (ref_log && user_data) {<br>@@ -498,6 +496,8 @@<br> #endif<br> <br>  if (weakproxy) {<br>+             struct ao2_weakproxy cbs;<br>+<br>          if (current_value == 1) {<br>                     /* The only remaining reference is the one owned by the weak object */<br>                        struct astobj2 *internal_weakproxy;<br>@@ -508,8 +508,9 @@<br>                      internal_weakproxy->priv_data.weakptr = NULL;<br>                      obj->priv_data.weakptr = NULL;<br> <br>-                 /* Notify the subscribers that weakproxy now points to NULL. */<br>-                      weakproxy_run_callbacks(weakproxy);<br>+                  /* transfer list to local copy so callbacks are run with weakproxy unlocked. */<br>+                      cbs.destroyed_cb = weakproxy->destroyed_cb;<br>+                       AST_LIST_HEAD_INIT_NOLOCK(&weakproxy->destroyed_cb);<br> <br>                        /* weak is already unlinked from obj so this won't recurse */<br>                     ao2_ref(user_data, -1);<br>@@ -518,6 +519,14 @@<br>                 ao2_unlock(weakproxy);<br> <br>             if (current_value == 1) {<br>+                    struct ao2_weakproxy_notification *destroyed_cb;<br>+<br>+                  /* Notify the subscribers that weakproxy now points to NULL. */<br>+                      while ((destroyed_cb = AST_LIST_REMOVE_HEAD(&cbs.destroyed_cb, list))) {<br>+                         destroyed_cb->cb(weakproxy, destroyed_cb->data);<br>+                               ast_free(destroyed_cb);<br>+                      }<br>+<br>                  ao2_ref(weakproxy, -1);<br>               }<br>     }<br>@@ -795,16 +804,6 @@<br>       return obj;<br> }<br> <br>-<br>-static void weakproxy_run_callbacks(struct ao2_weakproxy *weakproxy)<br>-{<br>-     struct ao2_weakproxy_notification *destroyed_cb;<br>-<br>-  while ((destroyed_cb = AST_LIST_REMOVE_HEAD(&weakproxy->destroyed_cb, list))) {<br>-               destroyed_cb->cb(weakproxy, destroyed_cb->data);<br>-               ast_free(destroyed_cb);<br>-      }<br>-}<br> <br> void *__ao2_weakproxy_alloc(size_t data_size, ao2_destructor_fn destructor_fn,<br>     const char *tag, const char *file, int line, const char *func)<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6733">change 6733</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6733"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ib167622a8a0f873fd73938f7611b2a5914308047 </div>
<div style="display:none"> Gerrit-Change-Number: 6733 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>