<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/11114">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">app.c:  Remove deletion of pool topic on mwi state delete<br><br>As part of an earlier voicemail refactor, ast_delete_mwi_state_full<br>was modified to remove the pool topic for a mailbox when the state<br>was deleted.  This was an attempt to prevent stale topics from<br>accumulating when app_voicemail was reloaded and a mailbox went<br>away.  Unfortunately because of the fact that when app_voicemail<br>reloads, ALL mailboxes are deleted then only current ones recreated,<br>topics were being removed from the pool that still had subscribers<br>on them, then recreated as new topics of the same name.  So now<br>modules like res_pjsip_mwi are listening on a topic that will<br>never receive any messages because app_voicemail is publishing on<br>a different topic that happens to have the same name.  The solutiuon<br>to this is not easy and given that accumulating topics for<br>deleted mailboxes is less evil that not sending NOTIFYs...<br><br>* Removed the call to stasis_topic_pool_delete_topic in<br>  ast_delete_mwi_state_full.<br><br>Also:<br><br>* Fixed a topic reference leak in res_pjsip_mwi<br>  mwi_stasis_subscription_alloc.<br><br>* Added some debugging to mwi_stasis_subscription_alloc,<br>  stasis_topic_create, and topic_dtor.<br><br>ASTERISK-28306<br>Reported-by: Jared Hull<br><br>Change-Id: Id7da0990b3ac4be4b58491536b35f41291247b27<br>---<br>M main/app.c<br>M main/stasis.c<br>M res/res_pjsip_mwi.c<br>3 files changed, 8 insertions(+), 4 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/14/11114/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/main/app.c b/main/app.c</span><br><span>index bc28ef6..a6b5e18 100644</span><br><span>--- a/main/app.c</span><br><span>+++ b/main/app.c</span><br><span>@@ -3356,8 +3356,6 @@</span><br><span>                stasis_publish(mailbox_specific_topic, clear_msg);</span><br><span>   }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   stasis_topic_pool_delete_topic(mwi_topic_pool, stasis_topic_name(mailbox_specific_topic));</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>   ao2_cleanup(clear_msg);</span><br><span>      return 0;</span><br><span> }</span><br><span>diff --git a/main/stasis.c b/main/stasis.c</span><br><span>index e464101..c70e59d 100644</span><br><span>--- a/main/stasis.c</span><br><span>+++ b/main/stasis.c</span><br><span>@@ -397,6 +397,7 @@</span><br><span> </span><br><span>    AST_VECTOR_FREE(&topic->subscribers);</span><br><span>         AST_VECTOR_FREE(&topic->upstream_topics);</span><br><span style="color: hsl(120, 100%, 40%);">+      ast_debug(1, "Topic '%s': %p destroyed\n", topic->name, topic);</span><br><span> </span><br><span> #ifdef AST_DEVMODE</span><br><span>       if (topic->statistics) {</span><br><span>@@ -449,6 +450,8 @@</span><br><span>    strcpy(topic->name, name); /* SAFE */</span><br><span>     res |= AST_VECTOR_INIT(&topic->subscribers, INITIAL_SUBSCRIBERS_MAX);</span><br><span>         res |= AST_VECTOR_INIT(&topic->upstream_topics, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+    ast_debug(1, "Topic '%s': %p created\n", topic->name, topic);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #ifdef AST_DEVMODE</span><br><span>   topic->statistics = stasis_topic_statistics_create(name);</span><br><span>         if (!topic->name || !topic->statistics || res)</span><br><span>diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c</span><br><span>index 5622515..1eb6129 100644</span><br><span>--- a/res/res_pjsip_mwi.c</span><br><span>+++ b/res/res_pjsip_mwi.c</span><br><span>@@ -259,10 +259,13 @@</span><br><span>        /* Safe strcpy */</span><br><span>    strcpy(mwi_stasis_sub->mailbox, mailbox);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        ast_debug(3, "Creating stasis MWI subscription to mailbox %s for endpoint %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">-              mailbox, mwi_sub->id);</span><br><span style="color: hsl(120, 100%, 40%);">+     ast_debug(3, "Creating stasis MWI subscription to mailbox %s for endpoint %s.  Topic: '%s':%p %d\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                mailbox, mwi_sub->id, stasis_topic_name(topic), topic, (int)ao2_ref(topic, 0));</span><br><span>   ao2_ref(mwi_sub, +1);</span><br><span>        mwi_stasis_sub->stasis_sub = stasis_subscribe_pool(topic, mwi_stasis_cb, mwi_sub);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The subscribe steals the topic ref */</span><br><span style="color: hsl(120, 100%, 40%);">+      ao2_ref(topic, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>        if (!mwi_stasis_sub->stasis_sub) {</span><br><span>                /* Failed to subscribe. */</span><br><span>           ao2_ref(mwi_stasis_sub, -1);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/11114">change 11114</a>. To unsubscribe, or for help writing mail filters, 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/c/asterisk/+/11114"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-Change-Id: Id7da0990b3ac4be4b58491536b35f41291247b27 </div>
<div style="display:none"> Gerrit-Change-Number: 11114 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>