<p>George Joseph <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/11116">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Benjamin Keith Ford: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
George Joseph: Approved for Submit
</div><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>* Fixed a topic reference leak in an error path in<br> internal_stasis_subscribe.<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;"><span>diff --git a/main/app.c b/main/app.c</span><br><span>index e8a4d2f..d272e40 100644</span><br><span>--- a/main/app.c</span><br><span>+++ b/main/app.c</span><br><span>@@ -3337,8 +3337,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, mwi_state->uniqueid);</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 5f4a147..7dd3893 100644</span><br><span>--- a/main/stasis.c</span><br><span>+++ b/main/stasis.c</span><br><span>@@ -400,6 +400,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>@@ -454,6 +455,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(topic);</span><br><span> if (!topic->name || !topic->statistics || res)</span><br><span>@@ -752,6 +755,7 @@</span><br><span> </span><br><span> if (topic_add_subscription(topic, sub) != 0) {</span><br><span> ao2_ref(sub, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(topic, -1);</span><br><span> </span><br><span> return NULL;</span><br><span> }</span><br><span>diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c</span><br><span>index eeb8a18..72f8b59 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,12 @@</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%);">+ 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/+/11116">change 11116</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/+/11116"/><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-Change-Id: Id7da0990b3ac4be4b58491536b35f41291247b27 </div>
<div style="display:none"> Gerrit-Change-Number: 11116 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua C. Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>