[Asterisk-code-review] app voicemail: Remove need to subscribe to stasis (asterisk[16])

Joshua Colp asteriskteam at digium.com
Thu Sep 20 04:53:44 CDT 2018


Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/10134 )

Change subject: app_voicemail: Remove need to subscribe to stasis
......................................................................

app_voicemail: Remove need to subscribe to stasis

app_voicemail was using the stasis cache to build and maintain a
list of mailboxes that had subscribers.  It then used this list
to determine if a mailbox should be polled for new messages if
polling was enabled.  For this to work, stasis had to cache every
subscription and unsubscription to the mailbox which caused a lot of
overhead, both cpu and memory related.

Since polling is only required when changes are being made to
mailboxes outside of app_voicemail and since the number of mailboxes
that don't have any subscribers is likely to be very low, all
mailboxes are now polled instead of just the ones with subscribers.

This paves the way for disabling the caching of stasis subscription
change messages.

Also fixed cleanup in some of the unit tests that not only left
test users in the users list but also caused segfaults if the tests
were run more than once.

ASTERISK-27121

Change-Id: I5cceb737246949f9782955c64425b8bd25a9e9ee
---
M apps/app_voicemail.c
M configs/samples/voicemail.conf.sample
2 files changed, 241 insertions(+), 280 deletions(-)

Approvals:
  Sean Bright: Looks good to me, but someone else must approve
  Richard Mudgett: Looks good to me, approved
  Joshua Colp: Approved for Submit



diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index c6e501c..c5afb32 100644
--- a/apps/app_voicemail.c
+++ b/apps/app_voicemail.c
@@ -1009,42 +1009,24 @@
 static pthread_t poll_thread = AST_PTHREADT_NULL;
 static unsigned char poll_thread_run;
 
-/*! Subscription to MWI event subscription changes */
-static struct stasis_subscription *mwi_sub_sub;
-
 /*!
- * \brief An MWI subscription
+ * \brief A mailbox to be polled
  *
- * This is so we can keep track of which mailboxes are subscribed to.
  * This way, we know which mailboxes to poll when the pollmailboxes
  * option is being used.
  */
-struct mwi_sub {
-	AST_RWLIST_ENTRY(mwi_sub) entry;
+struct poll_state {
+	int marked_used;
 	int old_urgent;
 	int old_new;
 	int old_old;
-	char *uniqueid;
 	char mailbox[0];
 };
 
-struct mwi_sub_task {
-	const char *mailbox;
-	const char *context;
-	const char *uniqueid;
-};
-
-static void mwi_sub_task_dtor(struct mwi_sub_task *mwist)
-{
-	ast_free((void *) mwist->mailbox);
-	ast_free((void *) mwist->context);
-	ast_free((void *) mwist->uniqueid);
-	ast_free(mwist);
-}
-
-static struct ast_taskprocessor *mwi_subscription_tps;
-
-static AST_RWLIST_HEAD_STATIC(mwi_subs, mwi_sub);
+#define POLL_LIST_BUCKETS 511
+static struct ao2_container *poll_list;
+AO2_STRING_FIELD_HASH_FN(poll_state, mailbox);
+AO2_STRING_FIELD_CMP_FN(poll_state, mailbox);
 
 /* custom audio control prompts for voicemail playback */
 static char listen_control_forward_key[12];
@@ -1122,7 +1104,7 @@
 static int message_range_and_existence_check(struct vm_state *vms, const char *msg_ids [], size_t num_msgs, int *msg_nums, struct ast_vm_user *vmu);
 static void notify_new_state(struct ast_vm_user *vmu);
 static int append_vmu_info_astman(struct mansession *s, struct ast_vm_user *vmu, const char* event_name, const char* actionid);
-
+static int poll_mailbox(void *obj, void *arg, int flags);
 
 /*!
  * Place a message in the indicated folder
@@ -12310,6 +12292,59 @@
 	return 0;
 }
 
+static void poll_state_dtor(void *obj)
+{
+	struct poll_state *poll_state = obj;
+
+	ast_debug(3, "DTOR: Mailbox: %s New: %d  Old: %d  Urgent: %d  Marked Used: %d\n", poll_state->mailbox,
+		poll_state->old_new, poll_state->old_old, poll_state->old_urgent,
+		poll_state->marked_used);
+}
+
+static int mark_or_create_poll_state(struct ast_vm_user *vmu)
+{
+	size_t len;
+	struct poll_state *poll_state;
+	char *mailbox;
+
+	if (ast_strlen_zero(vmu->mailbox)) {
+		ast_log(LOG_ERROR, "Mailbox can't be empty\n");
+		return -1;
+	}
+
+	len = sizeof(vmu->mailbox) + sizeof(vmu->context) + sizeof('@') + 1;
+	mailbox = ast_alloca(len);
+	len = snprintf(mailbox, len, "%s%s%s",
+		vmu->mailbox,
+		ast_strlen_zero(vmu->context) ? "" : "@",
+		vmu->context);
+
+	len++; /* For NULL terminator */
+
+	poll_state = ao2_find(poll_list, mailbox, OBJ_SEARCH_KEY);
+	if (poll_state) {
+		poll_state->marked_used = 1;
+		ao2_ref(poll_state, -1);
+		return 0;
+	}
+
+	poll_state = ao2_alloc_options(len + sizeof(*poll_state), poll_state_dtor,
+		AO2_ALLOC_OPT_LOCK_NOLOCK);
+	if (!poll_state) {
+		return -1;
+	}
+	strcpy(poll_state->mailbox, mailbox); /* Safe */
+	poll_state->marked_used = 1;
+
+	ao2_link_flags(poll_list, poll_state, OBJ_NOLOCK);
+
+	poll_mailbox(poll_state, NULL, 0);
+
+	ao2_ref(poll_state, -1);
+
+	return 0;
+}
+
 static struct ast_vm_user *find_or_create(const char *context, const char *box)
 {
 	struct ast_vm_user *vmu;
@@ -12339,12 +12374,18 @@
 		}
 	}
 
-	if (!(vmu = ast_calloc(1, sizeof(*vmu))))
+	if (!(vmu = ast_calloc(1, sizeof(*vmu)))) {
 		return NULL;
+	}
 
 	ast_copy_string(vmu->context, context, sizeof(vmu->context));
 	ast_copy_string(vmu->mailbox, box, sizeof(vmu->mailbox));
 
+	if (mark_or_create_poll_state(vmu)) {
+		ast_free(vmu);
+		return NULL;
+	}
+
 	AST_LIST_INSERT_TAIL(&users, vmu, list);
 
 	return vmu;
@@ -12600,6 +12641,7 @@
 #endif
 
 	free_user(vmu);
+
 	return res ? AST_TEST_FAIL : AST_TEST_PASS;
 }
 #endif
@@ -13010,38 +13052,33 @@
 	AST_CLI_DEFINE(handle_voicemail_reload, "Reload voicemail configuration"),
 };
 
-static void poll_subscribed_mailbox(struct mwi_sub *mwi_sub)
+static int poll_mailbox(void *obj, void *arg, int flags)
 {
+	struct poll_state *poll_state = obj;
 	int new = 0, old = 0, urgent = 0;
 
-	inboxcount2(mwi_sub->mailbox, &urgent, &new, &old);
+
+	inboxcount2(poll_state->mailbox, &urgent, &new, &old);
+	ast_debug(4, "Polled mailbox '%s' urgent: %d  new: %d  old: %d\n",
+		poll_state->mailbox, urgent, new, old);
 
 #ifdef IMAP_STORAGE
 	if (imap_poll_logout) {
-		imap_logout(mwi_sub->mailbox);
+		imap_logout(poll_state->mailbox);
 	}
 #endif
 
-	if (urgent != mwi_sub->old_urgent || new != mwi_sub->old_new || old != mwi_sub->old_old) {
-		mwi_sub->old_urgent = urgent;
-		mwi_sub->old_new = new;
-		mwi_sub->old_old = old;
-		queue_mwi_event(NULL, mwi_sub->mailbox, urgent, new, old);
-		run_externnotify(NULL, mwi_sub->mailbox, NULL);
+	if (urgent != poll_state->old_urgent || new != poll_state->old_new || old != poll_state->old_old) {
+		ast_debug(4, "Notifying subscribers of mailbox '%s' urgent: %d  new: %d  old: %d\n",
+			poll_state->mailbox, urgent, new, old);
+		poll_state->old_urgent = urgent;
+		poll_state->old_new = new;
+		poll_state->old_old = old;
+		queue_mwi_event(NULL, poll_state->mailbox, urgent, new, old);
+		run_externnotify(NULL, poll_state->mailbox, NULL);
 	}
-}
 
-static void poll_subscribed_mailboxes(void)
-{
-	struct mwi_sub *mwi_sub;
-
-	AST_RWLIST_RDLOCK(&mwi_subs);
-	AST_RWLIST_TRAVERSE(&mwi_subs, mwi_sub, entry) {
-		if (!ast_strlen_zero(mwi_sub->mailbox)) {
-			poll_subscribed_mailbox(mwi_sub);
-		}
-	}
-	AST_RWLIST_UNLOCK(&mwi_subs);
+	return 0;
 }
 
 static void *mb_poll_thread(void *data)
@@ -13061,18 +13098,13 @@
 		if (!poll_thread_run)
 			break;
 
-		poll_subscribed_mailboxes();
+		ast_debug(3, "Polling mailboxes\n");
+		ao2_callback(poll_list, OBJ_NODATA | OBJ_MULTIPLE, poll_mailbox, NULL);
 	}
 
 	return NULL;
 }
 
-static void mwi_sub_destroy(struct mwi_sub *mwi_sub)
-{
-	ast_free(mwi_sub->uniqueid);
-	ast_free(mwi_sub);
-}
-
 #ifdef IMAP_STORAGE
 static void imap_logout(const char *mailbox_id)
 {
@@ -13108,155 +13140,22 @@
 	vmstate_delete(vms);
 }
 
+static int imap_close_subscribed_mailboxes_cb(void *obj, void *arg, int flags)
+{
+	struct poll_state *poll_state = obj;
+	imap_logout(poll_state->mailbox);
+
+	return 0;
+}
 static void imap_close_subscribed_mailboxes(void)
 {
-	struct mwi_sub *mwi_sub;
-
-	AST_RWLIST_RDLOCK(&mwi_subs);
-	AST_RWLIST_TRAVERSE(&mwi_subs, mwi_sub, entry) {
-		if (!ast_strlen_zero(mwi_sub->mailbox)) {
-			imap_logout(mwi_sub->mailbox);
-		}
-	}
-	AST_RWLIST_UNLOCK(&mwi_subs);
+	ao2_callback(poll_list, OBJ_NODATA | OBJ_MULTIPLE, imap_close_subscribed_mailboxes_cb, NULL);
 }
 #endif
 
-static int handle_unsubscribe(void *datap)
-{
-	struct mwi_sub *mwi_sub;
-	char *uniqueid = datap;
-
-	AST_RWLIST_WRLOCK(&mwi_subs);
-	AST_RWLIST_TRAVERSE_SAFE_BEGIN(&mwi_subs, mwi_sub, entry) {
-		if (!strcmp(mwi_sub->uniqueid, uniqueid)) {
-			AST_LIST_REMOVE_CURRENT(entry);
-			/* Don't break here since a duplicate uniqueid
-			 * may have been added as a result of a cache dump. */
-#ifdef IMAP_STORAGE
-			imap_logout(mwi_sub->mailbox);
-#endif
-			mwi_sub_destroy(mwi_sub);
-		}
-	}
-	AST_RWLIST_TRAVERSE_SAFE_END
-	AST_RWLIST_UNLOCK(&mwi_subs);
-
-	ast_free(uniqueid);
-	return 0;
-}
-
-static int handle_subscribe(void *datap)
-{
-	unsigned int len;
-	struct mwi_sub *mwi_sub;
-	struct mwi_sub_task *p = datap;
-
-	len = sizeof(*mwi_sub) + 1;
-	if (!ast_strlen_zero(p->mailbox))
-		len += strlen(p->mailbox);
-
-	if (!ast_strlen_zero(p->context))
-		len += strlen(p->context) + 1; /* Allow for seperator */
-
-	if (!(mwi_sub = ast_calloc(1, len)))
-		return -1;
-
-	mwi_sub->uniqueid = ast_strdup(p->uniqueid);
-	if (!ast_strlen_zero(p->mailbox))
-		strcpy(mwi_sub->mailbox, p->mailbox);
-
-	if (!ast_strlen_zero(p->context)) {
-		strcat(mwi_sub->mailbox, "@");
-		strcat(mwi_sub->mailbox, p->context);
-	}
-
-	AST_RWLIST_WRLOCK(&mwi_subs);
-	AST_RWLIST_INSERT_TAIL(&mwi_subs, mwi_sub, entry);
-	AST_RWLIST_UNLOCK(&mwi_subs);
-	mwi_sub_task_dtor(p);
-	poll_subscribed_mailbox(mwi_sub);
-	return 0;
-}
-
-static void mwi_unsub_event_cb(struct stasis_subscription_change *change)
-{
-	char *uniqueid = ast_strdup(change->uniqueid);
-
-	if (!uniqueid) {
-		ast_log(LOG_ERROR, "Unable to allocate memory for uniqueid\n");
-		return;
-	}
-
-	if (ast_taskprocessor_push(mwi_subscription_tps, handle_unsubscribe, uniqueid) < 0) {
-		ast_free(uniqueid);
-	}
-}
-
-static void mwi_sub_event_cb(struct stasis_subscription_change *change)
-{
-	struct mwi_sub_task *mwist;
-	char *context;
-	char *mailbox;
-
-	mwist = ast_calloc(1, (sizeof(*mwist)));
-	if (!mwist) {
-		return;
-	}
-
-	if (separate_mailbox(ast_strdupa(stasis_topic_name(change->topic)), &mailbox, &context)) {
-		ast_free(mwist);
-		return;
-	}
-
-	mwist->mailbox = ast_strdup(mailbox);
-	mwist->context = ast_strdup(context);
-	mwist->uniqueid = ast_strdup(change->uniqueid);
-
-	if (ast_taskprocessor_push(mwi_subscription_tps, handle_subscribe, mwist) < 0) {
-		mwi_sub_task_dtor(mwist);
-	}
-}
-
-static void mwi_event_cb(void *userdata, struct stasis_subscription *sub, struct stasis_message *msg)
-{
-	struct stasis_subscription_change *change;
-	/* Only looking for subscription change notices here */
-	if (stasis_message_type(msg) != stasis_subscription_change_type()) {
-		return;
-	}
-
-	change = stasis_message_data(msg);
-	if (change->topic == ast_mwi_topic_all()) {
-		return;
-	}
-
-	if (!strcmp(change->description, "Subscribe")) {
-		mwi_sub_event_cb(change);
-	} else if (!strcmp(change->description, "Unsubscribe")) {
-		mwi_unsub_event_cb(change);
-	}
-}
-
-static int dump_cache(void *obj, void *arg, int flags)
-{
-	struct stasis_message *msg = obj;
-	mwi_event_cb(NULL, NULL, msg);
-	return 0;
-}
-
 static void start_poll_thread(void)
 {
 	int errcode;
-	mwi_sub_sub = stasis_subscribe(ast_mwi_topic_all(), mwi_event_cb, NULL);
-
-	if (mwi_sub_sub) {
-		struct ao2_container *cached = stasis_cache_dump(ast_mwi_state_cache(), stasis_subscription_change_type());
-		if (cached) {
-			ao2_callback(cached, OBJ_MULTIPLE | OBJ_NODATA, dump_cache, NULL);
-		}
-		ao2_cleanup(cached);
-	}
 
 	poll_thread_run = 1;
 
@@ -13269,8 +13168,6 @@
 {
 	poll_thread_run = 0;
 
-	mwi_sub_sub = stasis_unsubscribe_and_join(mwi_sub_sub);
-
 	ast_mutex_lock(&poll_lock);
 	ast_cond_signal(&poll_cond);
 	ast_mutex_unlock(&poll_lock);
@@ -13401,38 +13298,50 @@
 
 }
 
-static int manager_voicemail_refresh(struct mansession *s, const struct message *m)
+struct refresh_data {
+	const char *context;
+	const char *mailbox;
+};
+
+static int refresh_match(void *obj, void *arg, int gflags)
 {
-	const char *context = astman_get_header(m, "Context");
-	const char *mailbox = astman_get_header(m, "Mailbox");
-	struct mwi_sub *mwi_sub;
+	struct poll_state *poll_state = obj;
+	struct refresh_data *data = arg;
 	const char *at;
 
-	AST_RWLIST_RDLOCK(&mwi_subs);
-	AST_RWLIST_TRAVERSE(&mwi_subs, mwi_sub, entry) {
-		if (!ast_strlen_zero(mwi_sub->mailbox)) {
-			if (
-				/* First case: everything matches */
-				(ast_strlen_zero(context) && ast_strlen_zero(mailbox)) ||
-				/* Second case: match the mailbox only */
-				(ast_strlen_zero(context) && !ast_strlen_zero(mailbox) &&
-					(at = strchr(mwi_sub->mailbox, '@')) &&
-					strncmp(mailbox, mwi_sub->mailbox, at - mwi_sub->mailbox) == 0) ||
-				/* Third case: match the context only */
-				(!ast_strlen_zero(context) && ast_strlen_zero(mailbox) &&
-					(at = strchr(mwi_sub->mailbox, '@')) &&
-					strcmp(context, at + 1) == 0) ||
-				/* Final case: match an exact specified mailbox */
-				(!ast_strlen_zero(context) && !ast_strlen_zero(mailbox) &&
-					(at = strchr(mwi_sub->mailbox, '@')) &&
-					strncmp(mailbox, mwi_sub->mailbox, at - mwi_sub->mailbox) == 0 &&
-					strcmp(context, at + 1) == 0)
-			) {
-				poll_subscribed_mailbox(mwi_sub);
-			}
+	if (!ast_strlen_zero(poll_state->mailbox)) {
+		if (
+			/* First case: everything matches */
+			(ast_strlen_zero(data->context) && ast_strlen_zero(data->mailbox)) ||
+			/* Second case: match the mailbox only */
+			(ast_strlen_zero(data->context) && !ast_strlen_zero(data->mailbox) &&
+				(at = strchr(poll_state->mailbox, '@')) &&
+				strncmp(data->mailbox, poll_state->mailbox, at - poll_state->mailbox) == 0) ||
+			/* Third case: match the context only */
+			(!ast_strlen_zero(data->context) && ast_strlen_zero(data->mailbox) &&
+				(at = strchr(poll_state->mailbox, '@')) &&
+				strcmp(data->context, at + 1) == 0) ||
+			/* Final case: match an exact specified mailbox */
+			(!ast_strlen_zero(data->context) && !ast_strlen_zero(data->mailbox) &&
+				(at = strchr(poll_state->mailbox, '@')) &&
+				strncmp(data->mailbox, poll_state->mailbox, at - poll_state->mailbox) == 0 &&
+				strcmp(data->context, at + 1) == 0)
+		) {
+			poll_mailbox(poll_state, NULL, 0);
 		}
 	}
-	AST_RWLIST_UNLOCK(&mwi_subs);
+
+	return 0;
+}
+
+static int manager_voicemail_refresh(struct mansession *s, const struct message *m)
+{
+	struct refresh_data data = {
+		.context = astman_get_header(m, "Context"),
+		.mailbox = astman_get_header(m, "Mailbox"),
+	};
+
+	ao2_callback(poll_list, OBJ_NODATA | OBJ_MULTIPLE, refresh_match, (void *)&data);
 	astman_send_ack(s, m, "Refresh sent");
 	return RESULT_SUCCESS;
 }
@@ -13643,6 +13552,26 @@
 }
 #endif
 
+static int unmark_poll_state(void *obj, void*arg, int flags)
+{
+	struct poll_state *poll_state = obj;
+
+	poll_state->marked_used = 0;
+
+	return 0;
+}
+
+static int unmarked_poll_state(void *obj, void*arg, int flags)
+{
+	struct poll_state *poll_state = obj;
+
+	if (!poll_state->marked_used) {
+		return CMP_MATCH;
+	}
+
+	return 0;
+}
+
 static int actual_load_config(int reload, struct ast_config *cfg, struct ast_config *ucfg)
 {
 	struct ast_vm_user *current;
@@ -13653,8 +13582,6 @@
 	int x;
 	unsigned int tmpadsi[4];
 	char secretfn[PATH_MAX] = "";
-	long tps_queue_low;
-	long tps_queue_high;
 
 #ifdef IMAP_STORAGE
 	ast_copy_string(imapparentfolder, "\0", sizeof(imapparentfolder));
@@ -13670,6 +13597,17 @@
 	imap_close_subscribed_mailboxes();
 #endif
 
+	if (poll_thread != AST_PTHREADT_NULL) {
+		stop_poll_thread();
+	}
+
+	/*
+	 * Unmark all current poll states.  As mailboxes are (re)loaded,
+	 * the state will be marked as used.  Any not marked after the
+	 * (re)load, will be removed.
+	 */
+	ao2_callback(poll_list, OBJ_NODATA | OBJ_MULTIPLE, unmark_poll_state, NULL);
+
 	/* Free all the users structure */
 	free_vm_users();
 
@@ -14243,23 +14181,11 @@
 			pagerbody = ast_strdup(substitute_escapes(val));
 		}
 
-		tps_queue_high = AST_TASKPROCESSOR_HIGH_WATER_LEVEL;
 		if ((val = ast_variable_retrieve(cfg, "general", "tps_queue_high"))) {
-			if (sscanf(val, "%30ld", &tps_queue_high) != 1 || tps_queue_high <= 0) {
-				ast_log(AST_LOG_WARNING, "Invalid the taskprocessor high water alert trigger level '%s'\n", val);
-				tps_queue_high = AST_TASKPROCESSOR_HIGH_WATER_LEVEL;
-			}
+			ast_log(LOG_NOTICE, "Parameter tps_queue_high is obsolete and will be ignored\n");
 		}
-		tps_queue_low = -1;
 		if ((val = ast_variable_retrieve(cfg, "general", "tps_queue_low"))) {
-			if (sscanf(val, "%30ld", &tps_queue_low) != 1 ||
-				tps_queue_low < -1 || tps_queue_high < tps_queue_low) {
-				ast_log(AST_LOG_WARNING, "Invalid the taskprocessor low water clear alert level '%s'\n", val);
-				tps_queue_low = -1;
-			}
-		}
-		if (ast_taskprocessor_alert_set_levels(mwi_subscription_tps, tps_queue_low, tps_queue_high)) {
-			ast_log(AST_LOG_WARNING, "Failed to set alert levels for voicemail taskprocessor.\n");
+			ast_log(LOG_NOTICE, "Parameter tps_queue_low is obsolete and will be ignored\n");
 		}
 
 		/* load mailboxes from users.conf */
@@ -14329,15 +14255,16 @@
 		}
 
 		AST_LIST_UNLOCK(&users);
+		/* Remove any left over unmarked poll states */
+		ao2_callback(poll_list, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, unmarked_poll_state, NULL);
 
 		if (poll_mailboxes && poll_thread == AST_PTHREADT_NULL)
 			start_poll_thread();
-		if (!poll_mailboxes && poll_thread != AST_PTHREADT_NULL)
-			stop_poll_thread();;
 
 		return 0;
 	} else {
 		AST_LIST_UNLOCK(&users);
+		ao2_callback(poll_list, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, unmarked_poll_state, NULL);
 		ast_log(AST_LOG_WARNING, "Failed to load configuration file.\n");
 		return 0;
 	}
@@ -14558,7 +14485,6 @@
 {
 	int i, j, res = AST_TEST_PASS, syserr;
 	struct ast_vm_user *vmu;
-	struct ast_vm_user svm;
 	struct vm_state vms;
 #ifdef IMAP_STORAGE
 	struct ast_channel *chan = NULL;
@@ -14611,9 +14537,11 @@
 	}
 #endif
 
-	memset(&svm, 0, sizeof(svm));
-	if (!(vmu = find_user(&svm, testcontext, testmailbox)) &&
-		!(vmu = find_or_create(testcontext, testmailbox))) {
+	AST_LIST_LOCK(&users);
+	vmu = find_or_create(testcontext, testmailbox);
+	AST_LIST_UNLOCK(&users);
+
+	if (!vmu) {
 		ast_test_status_update(test, "Cannot create vmu structure\n");
 		ast_unreplace_sigchld();
 #ifdef IMAP_STORAGE
@@ -14641,8 +14569,8 @@
 #ifdef IMAP_STORAGE
 				chan = ast_channel_unref(chan);
 #endif
-				free_user(vmu);
-				return AST_TEST_FAIL;
+				res = AST_TEST_FAIL;
+				break;
 			}
 		}
 
@@ -14666,27 +14594,34 @@
 				res = AST_TEST_FAIL;
 			}
 		}
+		if (res) {
+			break;
+		}
 
 		new = old = urgent = 0;
 		if (ast_app_inboxcount(testspec, &new, &old)) {
 			ast_test_status_update(test, "inboxcount returned failure\n");
 			res = AST_TEST_FAIL;
+			break;
 		} else if (old != expected_results[i][3 + 0] || new != expected_results[i][3 + 2]) {
 			ast_test_status_update(test, "inboxcount(%s) returned old=%d (expected %d) and new=%d (expected %d)\n",
 				testspec, old, expected_results[i][3 + 0], new, expected_results[i][3 + 2]);
 			res = AST_TEST_FAIL;
+			break;
 		}
 
 		new = old = urgent = 0;
 		if (ast_app_inboxcount2(testspec, &urgent, &new, &old)) {
 			ast_test_status_update(test, "inboxcount2 returned failure\n");
 			res = AST_TEST_FAIL;
+			break;
 		} else if (old != expected_results[i][6 + 0] ||
 				urgent != expected_results[i][6 + 1] ||
 				   new != expected_results[i][6 + 2]    ) {
 			ast_test_status_update(test, "inboxcount2(%s) returned old=%d (expected %d), urgent=%d (expected %d), and new=%d (expected %d)\n",
 				testspec, old, expected_results[i][6 + 0], urgent, expected_results[i][6 + 1], new, expected_results[i][6 + 2]);
 			res = AST_TEST_FAIL;
+			break;
 		}
 
 		new = old = urgent = 0;
@@ -14697,6 +14632,9 @@
 				res = AST_TEST_FAIL;
 			}
 		}
+		if (res) {
+			break;
+		}
 	}
 
 	for (i = 0; i < 3; i++) {
@@ -14725,7 +14663,9 @@
 			syserr > 0 ? strerror(syserr) : "unable to fork()");
 	}
 
-	free_user(vmu);
+	/* restore config */
+	load_config(0); /* this might say "Failed to load configuration file." */
+
 	return res;
 }
 
@@ -14776,15 +14716,13 @@
 	snprintf(attach, sizeof(attach), "%s/sounds/en/tt-weasels", ast_config_AST_DATA_DIR);
 	snprintf(attach2, sizeof(attach2), "%s/sounds/en/tt-somethingwrong", ast_config_AST_DATA_DIR);
 
-	if (!(vmu = find_user(&vmus, testcontext, testmailbox)) &&
-		!(vmu = find_or_create(testcontext, testmailbox))) {
-		ast_test_status_update(test, "Cannot create vmu structure\n");
-		return AST_TEST_NOT_RUN;
-	}
+	AST_LIST_LOCK(&users);
+	vmu = find_or_create(testcontext, testmailbox);
+	AST_LIST_UNLOCK(&users);
 
-	if (vmu != &vmus && !(vmu = find_user(&vmus, testcontext, testmailbox))) {
-		ast_test_status_update(test, "Cannot find vmu structure?!!\n");
-		return AST_TEST_NOT_RUN;
+	if (!vmu) {
+		ast_test_status_update(test, "Cannot create vmu structure\n");
+		return AST_TEST_FAIL;
 	}
 
 	populate_defaults(vmu);
@@ -14835,7 +14773,10 @@
 		}
 	}
 	fclose(file);
-	free_user(vmu);
+
+	/* restore config */
+	load_config(0); /* this might say "Failed to load configuration file." */
+
 	return res;
 }
 
@@ -14909,7 +14850,7 @@
 #undef CHECK
 
 	/* restore config */
-	load_config(1); /* this might say "Failed to load configuration file." */
+	load_config(0); /* this might say "Failed to load configuration file." */
 
 cleanup:
 	unlink(config_filename);
@@ -14965,8 +14906,11 @@
 		return AST_TEST_FAIL;
 	}
 
-	if (!(vmu = find_user(NULL, testcontext, testmailbox)) &&
-			!(vmu = find_or_create(testcontext, testmailbox))) {
+	AST_LIST_LOCK(&users);
+	vmu = find_or_create(testcontext, testmailbox);
+	AST_LIST_UNLOCK(&users);
+
+	if (!vmu) {
 		ast_test_status_update(test, "Cannot create vmu structure\n");
 		chan = ast_channel_unref(chan);
 		return AST_TEST_FAIL;
@@ -14996,7 +14940,10 @@
 	}
 
 	chan = ast_channel_unref(chan);
-	free_user(vmu);
+
+	/* restore config */
+	load_config(0); /* this might say "Failed to load configuration file." */
+
 	return res;
 }
 #endif /* defined(TEST_FRAMEWORK) */
@@ -15062,10 +15009,13 @@
 #endif
 	ao2_ref(inprocess_container, -1);
 
-	if (poll_thread != AST_PTHREADT_NULL)
+	if (poll_thread != AST_PTHREADT_NULL) {
 		stop_poll_thread();
+	}
 
-	mwi_subscription_tps = ast_taskprocessor_unreference(mwi_subscription_tps);
+	ao2_container_unregister("voicemail_poll_list");
+	ao2_cleanup(poll_list);
+
 	ast_unload_realtime("voicemail");
 	ast_unload_realtime("voicemail_data");
 
@@ -15077,6 +15027,18 @@
 	return res;
 }
 
+static void print_poll_state(void *v_obj, void *where, ao2_prnt_fn *prnt)
+{
+	struct poll_state *poll_state = v_obj;
+
+	if (!poll_state) {
+		return;
+	}
+	prnt(where, "Mailbox: %s New: %d  Old: %d  Urgent: %d  Marked Used: %d", poll_state->mailbox,
+		poll_state->old_new, poll_state->old_old, poll_state->old_urgent,
+		poll_state->marked_used);
+}
+
 /*!
  * \brief Load the module
  *
@@ -15101,13 +15063,23 @@
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
+	poll_list = ao2_container_alloc(POLL_LIST_BUCKETS, poll_state_hash_fn, poll_state_cmp_fn);
+	if (!poll_list) {
+		ast_log(LOG_ERROR, "Unable to create poll_list container\n");
+		ao2_cleanup(inprocess_container);
+		return AST_MODULE_LOAD_DECLINE;
+	}
+	res = ao2_container_register("voicemail_poll_list", poll_list, print_poll_state);
+	if (res) {
+		ast_log(LOG_ERROR, "Unable to register poll_list container\n");
+		ao2_cleanup(inprocess_container);
+		ao2_cleanup(poll_list);
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
 	/* compute the location of the voicemail spool directory */
 	snprintf(VM_SPOOL_DIR, sizeof(VM_SPOOL_DIR), "%s/voicemail/", ast_config_AST_SPOOL_DIR);
 
-	if (!(mwi_subscription_tps = ast_taskprocessor_get("app_voicemail", 0))) {
-		ast_log(AST_LOG_WARNING, "failed to reference mwi subscription taskprocessor.  MWI will not work\n");
-	}
-
 	if ((res = load_config(0))) {
 		unload_module();
 		return AST_MODULE_LOAD_DECLINE;
diff --git a/configs/samples/voicemail.conf.sample b/configs/samples/voicemail.conf.sample
index e4130d3..43325a2 100644
--- a/configs/samples/voicemail.conf.sample
+++ b/configs/samples/voicemail.conf.sample
@@ -387,17 +387,6 @@
 ; defaults to being off
 ; backupdeleted=100
 
-; Asterisk Task Processor Queue Size
-; On heavy loaded system you may need to increase 'app_voicemail' taskprocessor queue.
-; If the taskprocessor queue size reached high water level, the alert is triggered.
-; If the alert is set then some modules (for example pjsip) slow down its production
-; until the alert is cleared.
-; The alert is cleared when taskprocessor queue size drops to the low water clear level.
-; The next options set taskprocessor queue levels for this module.
-; tps_queue_high=500	; Taskprocessor high water alert trigger level.
-; tps_queue_low=450	; Taskprocessor low water clear alert level.
-			; The default is -1 for 90% of high water level.
-
 [zonemessages]
 ; Users may be located in different timezones, or may have different
 ; message announcements for their introductory message when they enter

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: merged
Gerrit-Change-Id: I5cceb737246949f9782955c64425b8bd25a9e9ee
Gerrit-Change-Number: 10134
Gerrit-PatchSet: 5
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180920/6cceb75f/attachment-0001.html>


More information about the asterisk-code-review mailing list