[Asterisk-code-review] Work around thread safety issues in the PBX core. (asterisk[11])

Corey Farrell asteriskteam at digium.com
Tue May 19 00:04:33 CDT 2015


Corey Farrell has uploaded a new change for review.

  https://gerrit.asterisk.org/481

Change subject: Work around thread safety issues in the PBX core.
......................................................................

Work around thread safety issues in the PBX core.

The only safe use of ast_context_find is to check that it existed.  The
memory pointed to by the return value can be freed the moment
ast_context_find unlocks.

ast_context_find_or_create was not thread safe when operating on the global
contexts (active dialplan).  Multiple lock/unlock sections could allow other
threads to make changes between the steps.  The return value is still unsafe
beyond checking for success.

ast_context_delete was not thread safe when called with a non-NULL context.
Because ast_context_delete is called without contexts locked, the context
could have been freed already.

This change works around the issue by creating new functions:
* ast_context_find_nolock
* ast_context_find_or_create_nolock
* ast_context_destroy_nolock
* ast_context_destroy_by_name

The nolock variants do not lock the global contexts.  The caller is
responsible for running ast_wrlock_contexts or ast_rdlock_contexts.  The
ast_context_destroy_by_name runs the find and destroy operation within a
single lock.

This does not solve safety issues with the return value of the original
functions.  The original ast_context_find_or_create now uses a single
lock to perform all actions, so that function is safe when the return
value is only used to determine success or failure.

Places where ast_context_find was followed by ast_context_destroy have
been replaced with ast_context_destroy_by_name.  Most other callers of
ast_context_find and ast_context_find_or_create have been modified to
lock with ast_rdlock_contexts() or ast_wrlock_contexts() for the entire
operation, and use the new nolock variants of functions.  Any place where
the return value was just being checked for NULL or not NULL has been left
alone.

ASTERISK-25094 #close
Reported by: Corey Farrell

Change-Id: I1866b6787730c9c4f3f836b6133ffe9c820734fa
---
M apps/app_meetme.c
M channels/chan_iax2.c
M channels/chan_sip.c
M channels/chan_skinny.c
M include/asterisk/pbx.h
M main/features.c
M main/pbx.c
M pbx/pbx_config.c
M tests/test_gosub.c
M tests/test_pbx.c
10 files changed, 207 insertions(+), 103 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/81/481/1

diff --git a/apps/app_meetme.c b/apps/app_meetme.c
index 254f237..2d8d556 100644
--- a/apps/app_meetme.c
+++ b/apps/app_meetme.c
@@ -7347,14 +7347,13 @@
 	ao2_unlock(trunk);
 
 	if (!ast_strlen_zero(trunk->autocontext)) {
-		struct ast_context *context;
-		context = ast_context_find_or_create(NULL, NULL, trunk->autocontext, sla_registrar);
-		if (!context) {
+		if (!ast_context_find_or_create(NULL, NULL, trunk->autocontext, sla_registrar)) {
 			ast_log(LOG_ERROR, "Failed to automatically find or create "
 				"context '%s' for SLA!\n", trunk->autocontext);
 			return -1;
 		}
-		if (ast_add_extension2(context, 0 /* don't replace */, "s", 1,
+
+		if (ast_add_extension(trunk->autocontext, 0 /* don't replace */, "s", 1,
 			NULL, NULL, slatrunk_app, ast_strdup(trunk->name), ast_free_ptr, sla_registrar)) {
 			ast_log(LOG_ERROR, "Failed to automatically create extension "
 				"for trunk '%s'!\n", trunk->name);
@@ -7523,17 +7522,16 @@
 	ao2_unlock(station);
 
 	if (!ast_strlen_zero(station->autocontext)) {
-		struct ast_context *context;
 		struct sla_trunk_ref *trunk_ref;
-		context = ast_context_find_or_create(NULL, NULL, station->autocontext, sla_registrar);
-		if (!context) {
+
+		if (!ast_context_find_or_create(NULL, NULL, station->autocontext, sla_registrar)) {
 			ast_log(LOG_ERROR, "Failed to automatically find or create "
 				"context '%s' for SLA!\n", station->autocontext);
 			return -1;
 		}
 		/* The extension for when the handset goes off-hook.
 		 * exten => station1,1,SLAStation(station1) */
-		if (ast_add_extension2(context, 0 /* don't replace */, station->name, 1,
+		if (ast_add_extension(station->autocontext, 0 /* don't replace */, station->name, 1,
 			NULL, NULL, slastation_app, ast_strdup(station->name), ast_free_ptr, sla_registrar)) {
 			ast_log(LOG_ERROR, "Failed to automatically create extension "
 				"for trunk '%s'!\n", station->name);
@@ -7546,7 +7544,7 @@
 			snprintf(hint, sizeof(hint), "SLA:%s", exten);
 			/* Extension for this line button 
 			 * exten => station1_line1,1,SLAStation(station1_line1) */
-			if (ast_add_extension2(context, 0 /* don't replace */, exten, 1,
+			if (ast_add_extension(station->autocontext, 0 /* don't replace */, exten, 1,
 				NULL, NULL, slastation_app, ast_strdup(exten), ast_free_ptr, sla_registrar)) {
 				ast_log(LOG_ERROR, "Failed to automatically create extension "
 					"for trunk '%s'!\n", station->name);
@@ -7554,7 +7552,7 @@
 			}
 			/* Hint for this line button 
 			 * exten => station1_line1,hint,SLA:station1_line1 */
-			if (ast_add_extension2(context, 0 /* don't replace */, exten, PRIORITY_HINT,
+			if (ast_add_extension(station->autocontext, 0 /* don't replace */, exten, PRIORITY_HINT,
 				NULL, NULL, hint, NULL, NULL, sla_registrar)) {
 				ast_log(LOG_ERROR, "Failed to automatically create hint "
 					"for trunk '%s'!\n", station->name);
diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index e73b65f..f6708c6 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -14783,7 +14783,6 @@
 
 static int __unload_module(void)
 {
-	struct ast_context *con;
 	int x;
 
 	network_change_event_unsubscribe();
@@ -14863,9 +14862,7 @@
 	ao2_ref(callno_pool, -1);
 	ao2_ref(callno_pool_trunk, -1);
 
-	con = ast_context_find(regcontext);
-	if (con)
-		ast_context_destroy(con, "IAX2");
+	ast_context_destroy_by_name(regcontext, "IAX2");
 	ast_unload_realtime("iaxpeers");
 
 	iax2_tech.capabilities = ast_format_cap_destroy(iax2_tech.capabilities);
diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index c40a4c3..3637ad7 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -19509,8 +19509,7 @@
 			}
 			
 		}
-		if (stalecontext)
-			ast_context_destroy(ast_context_find(stalecontext), "SIP");
+		ast_context_destroy_by_name(stalecontext, "SIP");
 	}
 }
 
@@ -34869,7 +34868,6 @@
 {
 	struct sip_pvt *p;
 	struct sip_threadinfo *th;
-	struct ast_context *con;
 	struct ao2_iterator i;
 	int wait_count;
 
@@ -35045,10 +35043,7 @@
 	close(sipsock);
 	io_context_destroy(io);
 	ast_sched_context_destroy(sched);
-	con = ast_context_find(used_context);
-	if (con) {
-		ast_context_destroy(con, "SIP");
-	}
+	ast_context_destroy_by_name(used_context, "SIP");
 	ast_unload_realtime("sipregs");
 	ast_unload_realtime("sippeers");
 	ast_cc_monitor_unregister(&sip_cc_monitor_callbacks);
diff --git a/channels/chan_skinny.c b/channels/chan_skinny.c
index 58cb564..ef1fda2 100644
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -2037,8 +2037,7 @@
 			}
 			
 		}
-		if (stalecontext)
-			ast_context_destroy(ast_context_find(stalecontext), "Skinny");
+		ast_context_destroy_by_name(stalecontext, "Skinny");
 	}
 }
 
@@ -8014,7 +8013,6 @@
 	struct skinny_device *d;
 	struct skinny_line *l;
 	struct skinny_subchannel *sub;
-	struct ast_context *con;
 
 	ast_rtp_glue_unregister(&skinny_rtp_glue);
 	ast_channel_unregister(&skinny_tech);
@@ -8069,9 +8067,7 @@
 		ast_sched_context_destroy(sched);
 	}
 
-	con = ast_context_find(used_context);
-	if (con)
-		ast_context_destroy(con, "Skinny");
+	ast_context_destroy_by_name(used_context, "Skinny");
 
 	default_cap = ast_format_cap_destroy(default_cap);
 	skinny_tech.capabilities = ast_format_cap_destroy(skinny_tech.capabilities);
diff --git a/include/asterisk/pbx.h b/include/asterisk/pbx.h
index 47a787e..995c053 100644
--- a/include/asterisk/pbx.h
+++ b/include/asterisk/pbx.h
@@ -259,6 +259,30 @@
 int pbx_exec(struct ast_channel *c, struct ast_app *app, const char *data);
 
 /*!
+ * \brief Register a new context or find an existing one without locking.
+ *
+ * \param extcontexts pointer to the ast_context structure pointer
+ * \param exttable pointer to the hashtable that contains all the elements in extcontexts
+ * \param name name of the new context
+ * \param registrar registrar of the context
+ *
+ * This function allows you to play in two environments: the global contexts (active dialplan)
+ * or an external context set of your choosing. To act on the external set, make sure extcontexts
+ * and exttable are set; for the globals, make sure both extcontexts and exttable are NULL.
+ *
+ * This will first search for a context with your name.  If it exists already, it will not
+ * create a new one.  If it does not exist, it will create a new one with the given name
+ * and registrar.
+ *
+ * \return NULL on failure, and an ast_context structure on success
+ *
+ * \note This function must be called with a lock on contexts
+ * acquired by \ref ast_wrlock_contexts.
+ */
+struct ast_context *ast_context_find_or_create_nolock(struct ast_context **extcontexts,
+	struct ast_hashtab *exttable, const char *name, const char *registrar);
+
+/*!
  * \brief Register a new context or find an existing one
  *
  * \param extcontexts pointer to the ast_context structure pointer
@@ -275,6 +299,10 @@
  * and registrar.
  *
  * \return NULL on failure, and an ast_context structure on success
+ *
+ * \note The return value is not thread safe when working with the global contexts (active
+ * dialplan).  The moment it returns, another thread can replace the struct ast_context,
+ * freeing the returned pointer.  Use \ref ast_context_find_or_create_nolock instead.
  */
 struct ast_context *ast_context_find_or_create(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *name, const char *registrar);
 
@@ -291,7 +319,7 @@
 void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *registrar);
 
 /*!
- * \brief Destroy a context (matches the specified context (or ANY context if NULL)
+ * \brief Destroy a context without locking (matches the specified context or ANY context if NULL)
  *
  * \param con context to destroy
  * \param registrar who registered it
@@ -301,7 +329,50 @@
  *
  * \return nothing
  */
+void ast_context_destroy_nolock(struct ast_context *con, const char *registrar);
+
+/*!
+ * \brief Destroy a context by name
+ *
+ * \param context Name of the context to destroy
+ * \param registrar who registered it
+ *
+ * You can optionally leave out the registrar parameter.  It will find it
+ * based on the context name.
+ *
+ * \return nothing
+ */
+void ast_context_destroy_by_name(const char *context, const char *registrar);
+
+/*!
+ * \brief Destroy a context (matches the specified context or ANY context if NULL)
+ *
+ * \param con context to destroy
+ * \param registrar who registered it
+ *
+ * You can optionally leave out either parameter.  It will find it
+ * based on either the ast_context or the registrar name.
+ *
+ * \return nothing
+ *
+ * \note This method is only thread safe for NULL con.  For non-NULL con see
+ * \ref ast_context_destroy_nolock or ast_context_destroy_by_name.
+ */
 void ast_context_destroy(struct ast_context *con, const char *registrar);
+
+/*!
+ * \brief Find a context without locking.
+ *
+ * \param name name of the context to find
+ *
+ * Will search for the context with the given name.
+ *
+ * \return the ast_context on success, NULL on failure.
+ *
+ * \note This function must be called with a lock on contexts
+ * acquired by \ref ast_rdlock_contexts or \ref ast_wrlock_contexts.
+ */
+struct ast_context *ast_context_find_nolock(const char *name);
 
 /*!
  * \brief Find a context
@@ -311,6 +382,10 @@
  * Will search for the context with the given name.
  *
  * \return the ast_context on success, NULL on failure.
+ *
+ * \note The return value of this function is not thread safe.  The moment it returns,
+ * another thread can replace the struct ast_context, freeing the returned pointer.  Use
+ * \ref ast_context_find_nolock instead.
  */
 struct ast_context *ast_context_find(const char *name);
 
diff --git a/main/features.c b/main/features.c
index 82b7b2f..b60325c 100644
--- a/main/features.c
+++ b/main/features.c
@@ -5271,13 +5271,17 @@
 		}
 		if (manage_parked_call(pu, pfds, nfds, new_pfds, new_nfds, ms)) {
 			/* Parking is complete for this call so remove it from the parking lot. */
-			con = ast_context_find(pu->parkinglot->cfg.parking_con);
+			ast_wrlock_contexts();
+			con = ast_context_find_nolock(pu->parkinglot->cfg.parking_con);
 			if (con) {
-				if (ast_context_remove_extension2(con, pu->parkingexten, 1, NULL, 0)) {
+				if (ast_context_remove_extension2(con, pu->parkingexten, 1, NULL, 1)) {
 					ast_log(LOG_WARNING,
 						"Whoa, failed to remove the parking extension %s@%s!\n",
 						pu->parkingexten, pu->parkinglot->cfg.parking_con);
 				}
+			}
+			ast_unlock_contexts();
+			if (con) {
 				notify_metermaids(pu->parkingexten, pu->parkinglot->cfg.parking_con,
 					AST_DEVICE_NOT_INUSE);
 			} else {
@@ -5567,9 +5571,15 @@
 			callid = ast_callid_unref(callid);
 		}
 
-		con = ast_context_find(parkinglot->cfg.parking_con);
+		ast_wrlock_contexts();
+		con = ast_context_find_nolock(parkinglot->cfg.parking_con);
 		if (con) {
-			if (ast_context_remove_extension2(con, pu->parkingexten, 1, NULL, 0)) {
+			res = ast_context_remove_extension2(con, pu->parkingexten, 1, NULL, 1);
+		}
+		ast_unlock_contexts();
+
+		if (con) {
+			if (res) {
 				ast_log(LOG_WARNING, "Whoa, failed to remove the extension!\n");
 			} else {
 				notify_metermaids(pu->parkingexten, parkinglot->cfg.parking_con, AST_DEVICE_NOT_INUSE);
@@ -6978,7 +6988,6 @@
 {
 	struct parking_dp_context *old_ctx;
 	struct parking_dp_context *new_ctx;
-	struct ast_context *con;
 	int cmp;
 
 	old_ctx = AST_LIST_FIRST(old_map);
@@ -6992,10 +7001,7 @@
 		cmp = strcmp(old_ctx->context, new_ctx->context);
 		if (cmp < 0) {
 			/* New map does not have old map context. */
-			con = ast_context_find(old_ctx->context);
-			if (con) {
-				ast_context_destroy(con, registrar);
-			}
+			ast_context_destroy_by_name(old_ctx->context, registrar);
 			old_ctx = AST_LIST_NEXT(old_ctx, node);
 			continue;
 		}
@@ -7011,10 +7017,7 @@
 
 	/* Any old contexts left must be dead. */
 	for (; old_ctx; old_ctx = AST_LIST_NEXT(old_ctx, node)) {
-		con = ast_context_find(old_ctx->context);
-		if (con) {
-			ast_context_destroy(con, registrar);
-		}
+		ast_context_destroy_by_name(old_ctx->context, registrar);
 	}
 }
 
@@ -7245,7 +7248,6 @@
 
 int ast_features_reload(void)
 {
-	struct ast_context *con;
 	int res;
 
 	ast_mutex_lock(&features_reload_lock);/* Searialize reloading features.conf */
@@ -7258,10 +7260,7 @@
 	 * extension.  This is a small window of opportunity on an
 	 * execution chain that is not expected to happen very often.
 	 */
-	con = ast_context_find(parking_con_dial);
-	if (con) {
-		ast_context_destroy(con, registrar);
-	}
+	ast_context_destroy_by_name(parking_con_dial, registrar);
 
 	res = load_config(1);
 	ast_mutex_unlock(&features_reload_lock);
@@ -8687,9 +8686,15 @@
 		return -1;
 	}
 
-	con = ast_context_find(args->pu->parkinglot->cfg.parking_con);
+	ast_wrlock_contexts();
+	con = ast_context_find_nolock(args->pu->parkinglot->cfg.parking_con);
 	if (con) {
-		if (ast_context_remove_extension2(con, args->pu->parkingexten, 1, NULL, 0)) {
+		res = ast_context_remove_extension2(con, args->pu->parkingexten, 1, NULL, 1);
+	}
+	ast_unlock_contexts();
+
+	if (con) {
+		if (res) {
 			ast_log(LOG_WARNING, "Whoa, failed to remove the parking extension!\n");
 			res = -1;
 		} else {
diff --git a/main/pbx.c b/main/pbx.c
index 42e5f5c..a8df9aa 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -3120,27 +3120,37 @@
 	char name[256];
 };
 
-struct ast_context *ast_context_find(const char *name)
+struct ast_context *ast_context_find_nolock(const char *name)
 {
-	struct ast_context *tmp;
+	struct ast_context *tmp = NULL;
 	struct fake_context item;
 
 	if (!name) {
 		return NULL;
 	}
-	ast_rdlock_contexts();
+
 	if (contexts_table) {
 		ast_copy_string(item.name, name, sizeof(item.name));
-		tmp = ast_hashtab_lookup(contexts_table, &item);
-	} else {
-		tmp = NULL;
-		while ((tmp = ast_walk_contexts(tmp))) {
-			if (!strcasecmp(name, tmp->name)) {
-				break;
-			}
+		return ast_hashtab_lookup(contexts_table, &item);
+	}
+
+	while ((tmp = ast_walk_contexts(tmp))) {
+		if (!strcasecmp(name, tmp->name)) {
+			break;
 		}
 	}
+
+	return tmp;
+}
+
+struct ast_context *ast_context_find(const char *name)
+{
+	struct ast_context *tmp;
+
+	ast_rdlock_contexts();
+	tmp = ast_context_find_nolock(name);
 	ast_unlock_contexts();
+
 	return tmp;
 }
 
@@ -8759,15 +8769,14 @@
 	return tmp ? 0 : -1;
 }
 
-struct ast_context *ast_context_find_or_create(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *name, const char *registrar)
+struct ast_context *ast_context_find_or_create_nolock(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *name, const char *registrar)
 {
 	struct ast_context *tmp, **local_contexts;
 	struct fake_context search;
 	int length = sizeof(struct ast_context) + strlen(name) + 1;
 
-	if (!contexts_table) {
-		/* Protect creation of contexts_table from reentrancy. */
-		ast_wrlock_contexts();
+	ast_copy_string(search.name, name, sizeof(search.name));
+	if (!extcontexts) {
 		if (!contexts_table) {
 			contexts_table = ast_hashtab_create(17,
 				ast_hashtab_compare_contexts,
@@ -8776,15 +8785,8 @@
 				ast_hashtab_hash_contexts,
 				0);
 		}
-		ast_unlock_contexts();
-	}
-
-	ast_copy_string(search.name, name, sizeof(search.name));
-	if (!extcontexts) {
-		ast_rdlock_contexts();
 		local_contexts = &contexts;
 		tmp = ast_hashtab_lookup(contexts_table, &search);
-		ast_unlock_contexts();
 		if (tmp) {
 			tmp->refcount++;
 			return tmp;
@@ -8798,27 +8800,26 @@
 		}
 	}
 
-	if ((tmp = ast_calloc(1, length))) {
-		ast_rwlock_init(&tmp->lock);
-		ast_mutex_init(&tmp->macrolock);
-		strcpy(tmp->name, name);
-		tmp->root = NULL;
-		tmp->root_table = NULL;
-		tmp->registrar = ast_strdup(registrar);
-		tmp->includes = NULL;
-		tmp->ignorepats = NULL;
-		tmp->refcount = 1;
-	} else {
+	tmp = ast_calloc(1, length);
+	if (!tmp) {
 		ast_log(LOG_ERROR, "Danger! We failed to allocate a context for %s!\n", name);
 		return NULL;
 	}
 
+	ast_rwlock_init(&tmp->lock);
+	ast_mutex_init(&tmp->macrolock);
+	strcpy(tmp->name, name);
+	tmp->root = NULL;
+	tmp->root_table = NULL;
+	tmp->registrar = ast_strdup(registrar);
+	tmp->includes = NULL;
+	tmp->ignorepats = NULL;
+	tmp->refcount = 1;
+
 	if (!extcontexts) {
-		ast_wrlock_contexts();
 		tmp->next = *local_contexts;
 		*local_contexts = tmp;
 		ast_hashtab_insert_safe(contexts_table, tmp); /*put this context into the tree */
-		ast_unlock_contexts();
 		ast_debug(1, "Registered context '%s'(%p) in table %p registrar: %s\n", tmp->name, tmp, contexts_table, registrar);
 		ast_verb(3, "Registered extension context '%s'; registrar: %s\n", tmp->name, registrar);
 	} else {
@@ -8830,6 +8831,23 @@
 		ast_debug(1, "Registered context '%s'(%p) in local table %p; registrar: %s\n", tmp->name, tmp, exttable, registrar);
 		ast_verb(3, "Registered extension context '%s'; registrar: %s\n", tmp->name, registrar);
 	}
+	return tmp;
+}
+
+struct ast_context *ast_context_find_or_create(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *name, const char *registrar)
+{
+	struct ast_context *tmp;
+
+	if (!extcontexts) {
+		ast_wrlock_contexts();
+	}
+
+	tmp = ast_context_find_or_create_nolock(extcontexts, exttable, name, registrar);
+
+	if (!extcontexts) {
+		ast_unlock_contexts();
+	}
+
 	return tmp;
 }
 
@@ -9710,18 +9728,24 @@
 
 int ast_ignore_pattern(const char *context, const char *pattern)
 {
-	struct ast_context *con = ast_context_find(context);
+	int ret = 0;
+	struct ast_context *con;
 
+	ast_rdlock_contexts();
+	con = ast_context_find_nolock(context);
 	if (con) {
 		struct ast_ignorepat *pat;
 
 		for (pat = con->ignorepats; pat; pat = pat->next) {
-			if (ast_extension_match(pat->pattern, pattern))
-				return 1;
+			if (ast_extension_match(pat->pattern, pattern)) {
+				ret = 1;
+				break;
+			}
 		}
 	}
+	ast_unlock_contexts();
 
-	return 0;
+	return ret;
 }
 
 /*
@@ -11035,6 +11059,23 @@
 	}
 }
 
+void ast_context_destroy_nolock(struct ast_context *con, const char *registrar)
+{
+	__ast_context_destroy(contexts, contexts_table, con, registrar);
+}
+
+void ast_context_destroy_by_name(const char *context, const char *registrar)
+{
+	struct ast_context *con;
+
+	ast_wrlock_contexts();
+	con = ast_context_find_nolock(context);
+	if (con) {
+		ast_context_destroy_nolock(con, registrar);
+	}
+	ast_unlock_contexts();
+}
+
 void ast_context_destroy(struct ast_context *con, const char *registrar)
 {
 	ast_wrlock_contexts();
diff --git a/pbx/pbx_config.c b/pbx/pbx_config.c
index 7114bbc..2bf559f 100644
--- a/pbx/pbx_config.c
+++ b/pbx/pbx_config.c
@@ -97,14 +97,16 @@
 		return CLI_SHOWUSAGE;
 	}
 
-	con = ast_context_find(a->argv[3]);
+	ast_wrlock_contexts();
+	con = ast_context_find_nolock(a->argv[3]);
 
 	if (!con) {
-		ast_cli(a->fd, "There is no such context as '%s'\n",
-                        a->argv[3]);
-                return CLI_SUCCESS;
+		ast_unlock_contexts();
+		ast_cli(a->fd, "There is no such context as '%s'\n", a->argv[3]);
+		return CLI_SUCCESS;
 	} else {
-		ast_context_destroy(con, registrar);
+		ast_context_destroy_nolock(con, registrar);
+		ast_unlock_contexts();
 		ast_cli(a->fd, "Removing context '%s'\n",
 			a->argv[3]);
 		return CLI_SUCCESS;
diff --git a/tests/test_gosub.c b/tests/test_gosub.c
index 36573fa..3f6b3a7 100644
--- a/tests/test_gosub.c
+++ b/tests/test_gosub.c
@@ -42,10 +42,10 @@
 
 AST_TEST_DEFINE(test_gosub)
 {
+#define CONTEXT_NAME "tests_test_gosub_virtual_context"
 	int res = AST_TEST_PASS, i;
 	struct ast_channel *chan;
 	struct ast_str *str;
-	struct ast_context *con;
 	struct testplan {
 		const char *app;
 		const char *args;
@@ -119,14 +119,14 @@
 	}
 
 	/* Create our test dialplan */
-	if (!(con = ast_context_find_or_create(NULL, NULL, "tests_test_gosub_virtual_context", "test_gosub"))) {
+	if (!ast_context_find_or_create(NULL, NULL, CONTEXT_NAME, "test_gosub")) {
 		ast_test_status_update(test, "Unable to create test dialplan context");
 		ast_free(str);
 		ast_channel_unref(chan);
 		return AST_TEST_FAIL;
 	}
 
-	ast_add_extension2(con, 1, "s", 1, NULL, NULL, "NoOp", ast_strdup(""), ast_free_ptr, "test_gosub");
+	ast_add_extension(CONTEXT_NAME, 1, "s", 1, NULL, NULL, "NoOp", ast_strdup(""), ast_free_ptr, "test_gosub");
 
 	for (i = 0; i < ARRAY_LEN(testplan); i++) {
 		if (testplan[i].app == NULL) {
@@ -157,8 +157,8 @@
 
 	ast_free(str);
 	ast_channel_unref(chan);
-	ast_context_remove_extension2(con, "s", 1, NULL, 0);
-	ast_context_destroy(con, "test_gosub");
+	ast_context_remove_extension(CONTEXT_NAME, "s", 1, NULL);
+	ast_context_destroy(NULL, "test_gosub");
 
 	return res;
 }
diff --git a/tests/test_pbx.c b/tests/test_pbx.c
index 5e2e232..bb5d8e8 100644
--- a/tests/test_pbx.c
+++ b/tests/test_pbx.c
@@ -198,7 +198,6 @@
 	 */
 	struct {
 		const char * context_string;
-		struct ast_context *context;
 	} contexts[] = {
 		{ TEST_PATTERN, },
 		{ TEST_PATTERN_INCLUDE, },
@@ -267,7 +266,7 @@
 	 */
 
 	for (i = 0; i < ARRAY_LEN(contexts); ++i) {
-		if (!(contexts[i].context = ast_context_find_or_create(NULL, NULL, contexts[i].context_string, registrar))) {
+		if (!ast_context_find_or_create(NULL, NULL, contexts[i].context_string, registrar)) {
 			ast_test_status_update(test, "Failed to create context %s\n", contexts[i].context_string);
 			res = AST_TEST_FAIL;
 			goto cleanup;
@@ -319,11 +318,7 @@
 	}
 
 cleanup:
-	for (i = 0; i < ARRAY_LEN(contexts); ++i) {
-		if (contexts[i].context) {
-			ast_context_destroy(contexts[i].context, registrar);
-		}
-	}
+	ast_context_destroy(NULL, registrar);
 
 	return res;
 }

-- 
To view, visit https://gerrit.asterisk.org/481
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1866b6787730c9c4f3f836b6133ffe9c820734fa
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Corey Farrell <git at cfware.com>



More information about the asterisk-code-review mailing list