From 63d90c38ebe3e0774f372d745598d768490667ea Mon Sep 17 00:00:00 2001 From: George Joseph 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 e8a4d2f456..d272e40b42 100644 --- a/main/app.c +++ b/main/app.c @@ -3337,8 +3337,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 5f4a147dee..7dd3893d08 100644 --- a/main/stasis.c +++ b/main/stasis.c @@ -400,6 +400,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) { @@ -454,6 +455,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) @@ -752,6 +755,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 eeb8a18a63..72f8b59492 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);