From c87158f2da8c2acf3b85904a61abeef82d08a3c0 Mon Sep 17 00:00:00 2001
From: George Joseph <gjoseph@digium.com>
Date: Fri, 8 Mar 2019 08:40:38 -0700
Subject: [PATCH] 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.
* Fixed a topic reference leak in an error path in
internal_stasis_subscribe.
ASTERISK-28306
Reported-by: Jared Hull
Change-Id: Id7da0990b3ac4be4b58491536b35f41291247b27
---
main/app.c | 2 --
main/stasis.c | 4 ++++
res/res_pjsip_mwi.c | 6 ++++--
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/main/app.c b/main/app.c
index a73349b986..623843a13c 100644
--- a/main/app.c
+++ b/main/app.c
@@ -3356,8 +3356,6 @@ int ast_delete_mwi_state_full(const char *mailbox, const char *context, struct a
stasis_publish(mailbox_specific_topic, clear_msg);
}
- stasis_topic_pool_delete_topic(mwi_topic_pool, mwi_state->uniqueid);
-
ao2_cleanup(clear_msg);
return 0;
}
diff --git a/main/stasis.c b/main/stasis.c
index 9a1a5ad45d..3fee6ba3d8 100644
--- a/main/stasis.c
+++ b/main/stasis.c
@@ -402,6 +402,7 @@ static void topic_dtor(void *obj)
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) {
@@ -456,6 +457,8 @@ struct stasis_topic *stasis_topic_create(const char *name)
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(topic);
if (!topic->name || !topic->statistics || res)
@@ -754,6 +757,7 @@ struct stasis_subscription *internal_stasis_subscribe(
if (topic_add_subscription(topic, sub) != 0) {
ao2_ref(sub, -1);
+ ao2_ref(topic, -1);
return NULL;
}
diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index 56225155eb..7f86d981b6 100644
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -259,10 +259,12 @@ static struct mwi_stasis_subscription *mwi_stasis_subscription_alloc(const char
/* 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);
+ ao2_ref(topic, -1);
+
if (!mwi_stasis_sub->stasis_sub) {
/* Failed to subscribe. */
ao2_ref(mwi_stasis_sub, -1);
--
GitLab