[Asterisk-code-review] app.c: Remove deletion of pool topic on mwi state delete (...asterisk[13])

[George Joseph]

George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/11114


Change subject: app.c:  Remove deletion of pool topic on mwi state delete
......................................................................

app.c:  Remove deletion of pool topic on mwi state delete

As part of an earlier voicemail refactor, ast_delete_mwi_state_full
was modified to remove the pool topic for a mailbox when the state
was deleted.  This was an attempt to prevent stale topics from
accumulating when app_voicemail was reloaded and a mailbox went
away.  Unfortunately because of the fact that when app_voicemail
reloads, ALL mailboxes are deleted then only current ones recreated,
topics were being removed from the pool that still had subscribers
on them, then recreated as new topics of the same name.  So now
modules like res_pjsip_mwi are listening on a topic that will
never receive any messages because app_voicemail is publishing on
a different topic that happens to have the same name.  The solutiuon
to this is not easy and given that accumulating topics for
deleted mailboxes is less evil that not sending NOTIFYs...

* Removed the call to stasis_topic_pool_delete_topic in
  ast_delete_mwi_state_full.

Also:

* Fixed a topic reference leak in res_pjsip_mwi
  mwi_stasis_subscription_alloc.

* Added some debugging to mwi_stasis_subscription_alloc,
  stasis_topic_create, and topic_dtor.

ASTERISK-28306
Reported-by: Jared Hull

Change-Id: Id7da0990b3ac4be4b58491536b35f41291247b27
---
M main/app.c
M main/stasis.c
M res/res_pjsip_mwi.c
3 files changed, 8 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/14/11114/1

diff --git a/main/app.c b/main/app.c
index bc28ef6..a6b5e18 100644
--- a/main/app.c
+++ b/main/app.c
@@ -3356,8 +3356,6 @@
 		stasis_publish(mailbox_specific_topic, clear_msg);
 	}
 
-	stasis_topic_pool_delete_topic(mwi_topic_pool, stasis_topic_name(mailbox_specific_topic));
-
 	ao2_cleanup(clear_msg);
 	return 0;
 }
diff --git a/main/stasis.c b/main/stasis.c
index e464101..c70e59d 100644
--- a/main/stasis.c
+++ b/main/stasis.c
@@ -397,6 +397,7 @@
 
 	AST_VECTOR_FREE(&topic->subscribers);
 	AST_VECTOR_FREE(&topic->upstream_topics);
+	ast_debug(1, "Topic '%s': %p destroyed\n", topic->name, topic);
 
 #ifdef AST_DEVMODE
 	if (topic->statistics) {
@@ -449,6 +450,8 @@
 	strcpy(topic->name, name); /* SAFE */
 	res |= AST_VECTOR_INIT(&topic->subscribers, INITIAL_SUBSCRIBERS_MAX);
 	res |= AST_VECTOR_INIT(&topic->upstream_topics, 0);
+	ast_debug(1, "Topic '%s': %p created\n", topic->name, topic);
+
 #ifdef AST_DEVMODE
 	topic->statistics = stasis_topic_statistics_create(name);
 	if (!topic->name || !topic->statistics || res)
diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index 5622515..1eb6129 100644
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -259,10 +259,13 @@
 	/* Safe strcpy */
 	strcpy(mwi_stasis_sub->mailbox, mailbox);
 
-	ast_debug(3, "Creating stasis MWI subscription to mailbox %s for endpoint %s\n",
-		mailbox, mwi_sub->id);
+	ast_debug(3, "Creating stasis MWI subscription to mailbox %s for endpoint %s.  Topic: '%s':%p %d\n",
+		mailbox, mwi_sub->id, stasis_topic_name(topic), topic, (int)ao2_ref(topic, 0));
 	ao2_ref(mwi_sub, +1);
 	mwi_stasis_sub->stasis_sub = stasis_subscribe_pool(topic, mwi_stasis_cb, mwi_sub);
+	/* The subscribe steals the topic ref */
+	ao2_ref(topic, -1);
+
 	if (!mwi_stasis_sub->stasis_sub) {
 		/* Failed to subscribe. */
 		ao2_ref(mwi_stasis_sub, -1);

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11114
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Id7da0990b3ac4be4b58491536b35f41291247b27
Gerrit-Change-Number: 11114
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190308/113e6791/attachment.html>